Bug 113304

Summary: SocketStreamHandle (Chromium port) should fully use IPC window in send()
Product: WebKit Reporter: Takeshi Yoshino <tyoshino>
Component: WebCore Misc.Assignee: Takeshi Yoshino <tyoshino>
Status: RESOLVED FIXED    
Severity: Normal CC: ricea, tkent, toyoshim, webkit.review.bot, yhirano
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Takeshi Yoshino
Reported 2013-03-26 06:26:58 PDT
socket_stream of Chromium buffers send data up to 32KiB (exact) bytes. However, SocketStreamHandleInternal::send() method keeps in-flight send data not greater than m_maxPendingSendAllowed - 1 that is 32KiB - 1. This means that SocketStreamHandleInternal consumes the buffered data in SocketStreamHandleBase by 32KiB - 1. It makes memory copy operations unaligned unnecessarily. An experiment using the benchmark tool of pywebsocket and emulated 10ms RTT ($ sudo tc qdisc add dev lo root netem delay 10ms) showed this change improves send performance to ~3 times of current. Message| size | After Before KiB | (kB/s) (kB/s) -------+---------------------------------- 10 |50171.485 21000.82 50 |63327.149 22520.343 100 |61798.431 22664.896 500 |67949.569 23572.744 1000 |68817.204 23858.341 5000 |68817.204 23836.127 10000 |64160.401 23437.858 50000 |59362.319 22314.23 100000 |59918.081 23852.784
Attachments
Patch (2.21 KB, patch)
2013-03-26 06:31 PDT, Takeshi Yoshino 		no flags
DetailsFormatted DiffDiff
Patch (2.34 KB, patch)
2013-03-26 20:50 PDT, Takeshi Yoshino 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Takeshi Yoshino
Comment 1 2013-03-26 06:31:21 PDT
Created attachment 195075 [details] Patch
Takashi Toyoshima
Comment 2 2013-03-26 15:22:57 PDT
Comment on attachment 195075 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195075&action=review I agree with this change. > Source/WebCore/platform/network/chromium/SocketStreamHandle.cpp:80 > if (m_pendingAmountSent + len >= m_maxPendingSendAllowed) Nits: Now, this condition can be '>' instead of '>='.
Takashi Toyoshima
Comment 3 2013-03-26 15:25:56 PDT
+tkent for sanctifying to land this change.
Takeshi Yoshino
Comment 4 2013-03-26 20:50:29 PDT
Created attachment 195212 [details] Patch
Takeshi Yoshino
Comment 5 2013-03-26 20:51:48 PDT
Thanks for review, Takashi. (In reply to comment #2) > Nits: Now, this condition can be '>' instead of '>='. done
Kent Tamura
Comment 6 2013-03-26 21:41:44 PDT
Comment on attachment 195212 [details] Patch Looks ok
WebKit Review Bot
Comment 7 2013-03-26 22:05:41 PDT
Comment on attachment 195212 [details] Patch Clearing flags on attachment: 195212 Committed r146965: <http://trac.webkit.org/changeset/146965>
WebKit Review Bot
Comment 8 2013-03-26 22:05:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.