|Summary:||Disable Nagle algorithm on WebSocket implementation|
|Product:||WebKit||Reporter:||Takashi Toyoshima <toyoshim>|
|Component:||WebCore Misc.||Assignee:||Takashi Toyoshima <toyoshim>|
|Severity:||Normal||CC:||ap, bashi, rniwa, tkent, webkit.review.bot, yutak|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.8|
Description Takashi Toyoshima 2012-11-13 06:32:20 PST
WebSocket users need to send and receive small packets without latency. But, currently CF port has a flaky latency issue in some environment. This is because CF port doesn't disable Nagle. Combination of Nagle and delayed-ACK causes performance problem. I confirm performance regression happens on communicating from Japan to US using laptop machine over Wifi. I can see the same problem in Safari for OS X and iOS.
Comment 1 Takashi Toyoshima 2012-11-13 06:33:02 PST
FYI, Chrome disables Nagle and doesn't have this problem.
Comment 3 Build Bot 2012-11-13 07:11:18 PST
Comment on attachment 173883 [details] Patch Attachment 173883 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14820564
Comment 4 Alexey Proskuryakov 2012-11-13 10:31:45 PST
10>..\platform\network\cf\SocketStreamHandleCFNet.cpp(41) : fatal error C1083: Cannot open include file: 'netinet/in.h': No such file or directory
Comment 5 Alexey Proskuryakov 2012-11-13 10:51:20 PST
Is the issue you are seeing the one described here <http://www.stuartcheshire.org/papers/NagleDelayedAck/>? Can you please provide a more detailed description of the issue, and attach a tcpdump capture?
Comment 6 Takashi Toyoshima 2012-11-14 03:56:41 PST
Hi Alexey, I sent some data related to this problem in email directly.
Comment 8 Takashi Toyoshima 2012-11-27 06:47:17 PST
Created attachment 176258 [details] Patch Previous patch fails to set NODELAY.
Comment 9 Build Bot 2012-11-27 07:14:50 PST
Comment on attachment 176258 [details] Patch Attachment 176258 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15013102
Comment 10 Takashi Toyoshima 2012-12-06 06:06:18 PST
Created attachment 178004 [details] Patch Hopefully, this will fix cf/win build.
Comment 11 Takashi Toyoshima 2012-12-06 06:07:29 PST
Comment on attachment 178004 [details] Patch fix MIME Type and patch flag.
Comment 12 Build Bot 2012-12-06 06:43:29 PST
Comment on attachment 178004 [details] Patch Attachment 178004 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15152855
Comment 13 Alexey Proskuryakov 2012-12-06 09:56:06 PST
I think that we should use an SPI here: CFStreamCreatePairWithSocketToHost(0, host.get(), port(), &readStream, &writeStream); + CFWriteStreamSetProperty(writeStream, _kCFStreamSocketSetNoDelay, kCFBooleanTrue); and define the symbol at the top of the file: #if PLATFORM(WIN) #define EXTERN extern "C" __declspec(dllimport) #else #define EXTERN extern "C" #endif EXTERN const CFStringRef _kCFStreamSocketSetNoDelay;
Comment 14 Takashi Toyoshima 2012-12-10 21:21:20 PST
Created attachment 178705 [details] Patch Thanks. Your suggestion works fine!
Comment 15 Build Bot 2012-12-10 21:49:10 PST
Comment on attachment 178705 [details] Patch Attachment 178705 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15236855
Comment 16 Takashi Toyoshima 2012-12-11 00:45:39 PST
16> Creating library C:\cygwin\home\buildbot\WebKit\WebKitBuild\Debug\lib\WebKit.lib and object C:\cygwin\home\buildbot\WebKit\WebKitBuild\Debug\lib\WebKit.exp 16>WebCore.lib(SocketStreamHandleCFNet.obj) : error LNK2001: unresolved external symbol __imp___kCFStreamSocketSetNoDelay 16>C:\cygwin\home\buildbot\WebKit\WebKitBuild\Debug\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals Oops, stub lib doesn't have an entry for _kCFStreamSocketSetNoDelay seemingly. I'll try the 3rd patch with include wsock2.h.
Comment 17 Takashi Toyoshima 2012-12-20 01:25:04 PST
Created attachment 180294 [details] Patch I'm sorry but I still don't have perfect build environment for CF/WIN. So could you allow me to post a patch for try.
Comment 18 Build Bot 2012-12-20 05:12:18 PST
Comment on attachment 180294 [details] Patch Attachment 180294 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15406973
Comment 20 Build Bot 2012-12-20 06:43:29 PST
Comment on attachment 180331 [details] Patch Attachment 180331 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15456012
Comment 21 Takashi Toyoshima 2012-12-20 06:53:13 PST
Created attachment 180335 [details] Patch Hum... can I give up to support CF/Win...
Comment 22 Alexey Proskuryakov 2012-12-20 09:22:38 PST
Comment on attachment 180335 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180335&action=review > Source/WebCore/platform/network/cf/SocketStreamHandleCFNet.cpp:282 > + CFWriteStreamSetProperty(writeStream, _kCFStreamSocketSetNoDelay, kCFBooleanTrue); Please add a comment here: // <rdar://problem/12855587> _kCFStreamSocketSetNoDelay is not exported on Windows
Comment 24 WebKit Review Bot 2012-12-20 21:42:51 PST
Comment on attachment 180479 [details] Patch Rejecting attachment 180479 [details] from review queue. firstname.lastname@example.org does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your reviewer rights.
Comment 25 WebKit Review Bot 2012-12-20 22:08:26 PST
Comment on attachment 180479 [details] Patch Clearing flags on attachment: 180479 Committed r138348: <http://trac.webkit.org/changeset/138348>
Comment 26 WebKit Review Bot 2012-12-20 22:08:31 PST
All reviewed patches have been landed. Closing bug.
Comment 27 Ryosuke Niwa 2012-12-20 23:07:03 PST
This caused a build failure on Mac: http://build.webkit.org/builders/Apple%20Lion%20Debug%20%28Build%29/builds/9716/steps/compile-webkit/logs/stdio Undefined symbols for architecture x86_64: "__kCFStreamSocketSetNoDelay", referenced from: __ZN7WebCore18SocketStreamHandle13createStreamsEv in SocketStreamHandleCFNet.o ld: symbol(s) not found for architecture x86_64
Comment 28 Alexey Proskuryakov 2012-12-20 23:26:30 PST
Disabled this new code on Lion in r138351 as a build fix.
Comment 29 Takashi Toyoshima 2012-12-20 23:34:34 PST
Comment 30 Ryosuke Niwa 2012-12-20 23:43:29 PST
One more fix: http://trac.webkit.org/changeset/138352
Comment 31 Takashi Toyoshima 2012-12-20 23:53:57 PST
rniwa: Thanks! To my great relief, iOS supports _kCFStreamSocketSetNoDelay ;-)