[Webkit-unassigned] [Bug 79880] [WebSocket]Optimize the masking/unmasking operation by 64-bit data type

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79880


Zoltan Herczeg <zherczeg at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #130317|review?                     |review-
               Flag|                            |




--- Comment #16 from Zoltan Herczeg <zherczeg at webkit.org>  2012-03-06 02:24:55 PST ---
(From update of attachment 130317)
In general this looks well, and probably works well no other machines like ARM with NEON.

What I dont like is &base[offset] forms, please use base + offset instead of them.

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

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Remove this line. And please describe why is a win here as well. And add write some perf results.

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:113
> +static void maskFrameDataByUint64(char *src, size_t dataLength, const char *mask)

Please don't abbreaviate. Use source instead of src. Or buffer since it is also a destination...
And add an "inline" keyword. Otherwise this is probably not a win.

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:119
> +    if (dataLength > 0) {

WebKit prefers early return. So use:

if (datalength == 0)
    return;

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:123
> +            maskAlignedArray[i] = mask[index % maskingKeyWidthInBytes];

Is maskingKeyWidthInBytes is <= 8 and power of 2 all the time? We should add a COMPILE_ASSERT for it!

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:133
> +        for (size_t i = 0; i < dataLength % 8; i++)
> +             pSrc[i] ^= maskAlignedArray[i];

Just thinking about these fors...

dataLength &= ~0x7;
while (dataLength-- > 0)
    *pSrc++ ^= *maskAlignedArray++;

I think compiler like this better.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.