[Webkit-unassigned] [Bug 228240] 3.5 MB system-wide footprint impact due to thread-locals in libANGLE

[bugzilla-daemon at webkit.org]

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

Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ggaren at apple.com
 Attachment #434103|review+                     |review-
              Flags|                            |

--- Comment #5 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 434103
  --> https://bugs.webkit.org/attachment.cgi?id=434103
WIP

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

I appreciate Kenneth's review but I'm gonna be a little more explicit and say r- because I see a bug.

> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:47
> +    static dispatch_once_t once = 0;

Better to leave off the = 0 and accept whatever the platform says is the default linker-initialized value.

>> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:49
>> +    static bool initialized = false;
>> +    if (!initialized) {
> 
> Is it necessary to do this double-checking? The documentation indicates that the correct usage is just to call dispatch_once unconditionally.
> https://developer.apple.com/documentation/dispatch/1447169-dispatch_once

Kenneth is right. This is a bug. Because 'initialized' lacks any synchronization or memory barrier, it can read as true when it is still logically false.

I suggest removing the initialized variable and always calling dispatch_once. dispatch_once is fast (and inlined) in the "already dispatched" case.

>> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:119
>> +    if (!initialized) {
> 
> Same question about the double-checked initialization, though I do see this is on the hot path.

Same comments as above.

> Source/WebKit/NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.mm:282
> +#if defined(NW_HAS_SHARE_LISTENER_PORT) && NW_HAS_SHARE_LISTENER_PORT && 0

Revert?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210723/6916e7e1/attachment.htm>