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

Description Takeshi Yoshino 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
Comment 1 Takeshi Yoshino 2013-03-26 06:31:21 PDT
Created attachment 195075 [details]
Patch
Comment 2 Takashi Toyoshima 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 '>='.
Comment 3 Takashi Toyoshima 2013-03-26 15:25:56 PDT
+tkent for sanctifying to land this change.
Comment 4 Takeshi Yoshino 2013-03-26 20:50:29 PDT
Created attachment 195212 [details]
Patch
Comment 5 Takeshi Yoshino 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
Comment 6 Kent Tamura 2013-03-26 21:41:44 PDT
Comment on attachment 195212 [details]
Patch

Looks ok
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2013-03-26 22:05:46 PDT
All reviewed patches have been landed.  Closing bug.