<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body><span class="vcard"><a class="email" href="mailto:kbr@google.com" title="Kenneth Russell <kbr@google.com>"> <span class="fn">Kenneth Russell</span></a>
</span> changed
          <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - 3.5 MB system-wide footprint impact due to thread-locals in libANGLE"
   href="https://bugs.webkit.org/show_bug.cgi?id=228240">bug 228240</a>
          <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #434103 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - 3.5 MB system-wide footprint impact due to thread-locals in libANGLE"
   href="https://bugs.webkit.org/show_bug.cgi?id=228240#c4">Comment # 4</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - 3.5 MB system-wide footprint impact due to thread-locals in libANGLE"
   href="https://bugs.webkit.org/show_bug.cgi?id=228240">bug 228240</a>
              from <span class="vcard"><a class="email" href="mailto:kbr@google.com" title="Kenneth Russell <kbr@google.com>"> <span class="fn">Kenneth Russell</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=434103&action=diff" name="attach_434103" title="WIP">attachment 434103</a> <a href="attachment.cgi?id=434103&action=edit" title="WIP">[details]</a></span>
WIP

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=434103&action=review">https://bugs.webkit.org/attachment.cgi?id=434103&action=review</a>

Looks good overall especially if this significantly reduces binary size, though it's unfortunate we can't use the built-in mechanisms.

A few questions / comments, but to unblock this work, r+ now.

<span class="quote">> Source/ThirdParty/ANGLE/src/libANGLE/Context.cpp:302
> +void FreeCurrentValidContext()</span >

This and the function above are a little confusingly named. Could you consider:

  InitializeCurrentValidContextTLS
  FreeCurrentValidContextTLS
?

I also noticed that FreeCurrentValidContext isn't called from anywhere - which is fine because it can really only be called upon unloading of ANGLE or application shutdown. Should a comment be added about when it could plausibly be used?

<span class="quote">> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:49
> +    if (!initialized) {</span >

Is it necessary to do this double-checking? The documentation indicates that the correct usage is just to call dispatch_once unconditionally.
<a href="https://developer.apple.com/documentation/dispatch/1447169-dispatch_once">https://developer.apple.com/documentation/dispatch/1447169-dispatch_once</a>

<span class="quote">> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:99
> +bool InitializeCurrentThreadTLS()</span >

The naming convention between the CurrentThread and CurrentValidContext should match. So maybe here:

InitializeCurrentThreadTLSIndex
FreeCurrentThreadTLSIndex
GetCurrentThreadTLS
SetCurrentThreadTLS

and use TLSIndex / TLS in the CurrentValidContext functions above.

<span class="quote">> Source/ThirdParty/ANGLE/src/libGLESv2/global_state.cpp:119
> +    if (!initialized) {</span >

Same question about the double-checked initialization, though I do see this is on the hot path.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>