Bug 97237

Summary: [WebSocket] Receiving a large message is really slow
Product: WebKit Reporter: evan.exe
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bashi, webkit.review.bot, yutak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Vector<char> instead of char array in WebSocketChannel none

Description evan.exe 2012-09-20 10:52:41 PDT
It takes around 6 seconds for WebKit to receive single a 5 MB message over a WebSocket on localhost. It takes Firefox around 0.125 seconds in comparison. The test code is available at https://gist.github.com/3753757/.

I looked into it and from what I can tell, WebSocketChannel is receiving data from the socket in 1 KB increments and reallocating its entire buffer every time to store each new increment.
Comment 1 evan.exe 2012-09-20 16:35:56 PDT
Created attachment 165008 [details]
Vector<char> instead of char array in WebSocketChannel
Comment 2 Alexey Proskuryakov 2012-09-21 10:00:31 PDT
Comment on attachment 165008 [details]
Vector<char> instead of char array in WebSocketChannel

View in context: https://bugs.webkit.org/attachment.cgi?id=165008&action=review

Looks great to me. Marking r+ for now, will cq+ once EWS reports success.

In the future, please do mark patches r? and cq? as appropriate, so that they are visible in review queue.

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:394
> +        LOG(Network, "WebSocket buffer overflow (%lu+%lu)", static_cast<unsigned long>(m_buffer.size()), static_cast<unsigned long>(len));

Not related to this patch at all, but: this is a runtime condition, and should be logged to developer console, not to a debug channel.

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:405
> +    memmove(m_buffer.data(), m_buffer.data() + len, m_buffer.size() - len);
> +    m_buffer.resize(m_buffer.size() - len);

This looks suspicious. We are still doing lots of unnecessary copying.

You could have used Vector::remove() here - not that it would have improved performance characteristics, but it would be one line instead of three.
Comment 3 WebKit Review Bot 2012-09-21 10:28:02 PDT
Comment on attachment 165008 [details]
Vector<char> instead of char array in WebSocketChannel

Clearing flags on attachment: 165008

Committed r129239: <http://trac.webkit.org/changeset/129239>
Comment 4 WebKit Review Bot 2012-09-21 10:28:05 PDT
All reviewed patches have been landed.  Closing bug.