Disable Nagle algorithm on WebSocket implementation

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Takashi Toyoshima]

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.

[Takashi Toyoshima]

FYI, Chrome disables Nagle and doesn't have this problem.

[Takashi Toyoshima]

Created attachment 173883 [details]
Patch

[Build Bot]

Comment on attachment 173883 [details]
Patch

Attachment 173883 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14820564

[Alexey Proskuryakov]

10>..\platform\network\cf\SocketStreamHandleCFNet.cpp(41) : fatal error C1083: Cannot open include file: 'netinet/in.h': No such file or directory

[Alexey Proskuryakov]

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?

[Takashi Toyoshima]

Hi Alexey,

I sent some data related to this problem in email directly.

[Alexey Proskuryakov]

<rdar://problem/12706490>

[Takashi Toyoshima]

Created attachment 176258 [details]
Patch

Previous patch fails to set NODELAY.

[Build Bot]

Comment on attachment 176258 [details]
Patch

Attachment 176258 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15013102

[Takashi Toyoshima]

Created attachment 178004 [details]
Patch

Hopefully, this will fix cf/win build.

[Takashi Toyoshima]

Comment on attachment 178004 [details]
Patch

fix MIME Type and patch flag.

[Build Bot]

Comment on attachment 178004 [details]
Patch

Attachment 178004 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15152855

[Alexey Proskuryakov]

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;

[Takashi Toyoshima]

Created attachment 178705 [details]
Patch

Thanks.
Your suggestion works fine!

[Build Bot]

Comment on attachment 178705 [details]
Patch

Attachment 178705 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15236855

[Takashi Toyoshima]

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.

[Takashi Toyoshima]

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.

[Build Bot]

Comment on attachment 180294 [details]
Patch

Attachment 180294 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15406973

[Takashi Toyoshima]

Created attachment 180331 [details]
Patch

[Build Bot]

Comment on attachment 180331 [details]
Patch

Attachment 180331 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15456012

[Takashi Toyoshima]

Created attachment 180335 [details]
Patch

Hum... can I give up to support CF/Win...

[Alexey Proskuryakov]

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

[Takashi Toyoshima]

Created attachment 180479 [details]
Patch

[WebKit Review Bot]

Comment on attachment 180479 [details]
Patch

Rejecting attachment 180479 [details] from review queue.

[e-mail] 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.

[WebKit Review Bot]

Comment on attachment 180479 [details]
Patch

Clearing flags on attachment: 180479

Committed r138348: <http://trac.webkit.org/changeset/138348>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

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

[Alexey Proskuryakov]

Disabled this new code on Lion in r138351 as a build fix.

[Takashi Toyoshima]

ap: thanks!

[Ryosuke Niwa]

One more fix: http://trac.webkit.org/changeset/138352

[Takashi Toyoshima]

rniwa:
Thanks!
To my great relief, iOS supports _kCFStreamSocketSetNoDelay ;-)