Bug 215535 - Use std::call_once + LazyNeverDestroyed to initialize complex data structures
Summary: Use std::call_once + LazyNeverDestroyed to initialize complex data structures
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-15 00:56 PDT by Yusuke Suzuki
Modified: 2020-08-15 21:40 PDT (History)
11 users (show)

See Also:

Attachments
Patch (22.94 KB, patch)
2020-08-15 01:08 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (22.97 KB, patch)
2020-08-15 01:11 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (22.99 KB, patch)
2020-08-15 01:18 PDT, Yusuke Suzuki 		mark.lam: review+
Details | Formatted Diff | Diff
Patch (23.01 KB, patch)
2020-08-15 01:42 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-08-15 00:56:42 PDT 
Use std::call_once + LazyNeverDestroyed to initialize complex data structures
Comment 1 Yusuke Suzuki 2020-08-15 01:08:00 PDT 
Created attachment 406657 [details]
Patch
Comment 2 Yusuke Suzuki 2020-08-15 01:11:45 PDT 
Created attachment 406658 [details]
Patch
Comment 3 Yusuke Suzuki 2020-08-15 01:12:36 PDT 
<rdar://problem/66774266>
Comment 4 Yusuke Suzuki 2020-08-15 01:18:14 PDT 
Created attachment 406659 [details]
Patch
Comment 5 Mark Lam 2020-08-15 01:34:16 PDT 
Comment on attachment 406659 [details]
Patch

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

r=me if EWS bots are happy.

> Source/WTF/wtf/Logger.cpp:35
> +Lock loggerObserverLock { };

Don't need the { }.
Comment 6 Yusuke Suzuki 2020-08-15 01:42:36 PDT 
Created attachment 406660 [details]
Patch
Comment 7 Yusuke Suzuki 2020-08-15 11:41:02 PDT 
Committed r265735: <https://trac.webkit.org/changeset/265735>
Comment 8 Darin Adler 2020-08-15 12:00:53 PDT 
Comment on attachment 406660 [details]
Patch

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

> Source/WTF/wtf/Language.cpp:87
> -    static NeverDestroyed<Vector<String>> override;
> +    static LazyNeverDestroyed<Vector<String>> override;
> +    static std::once_flag onceKey;
> +    std::call_once(onceKey, [&] {
> +        override.construct();
> +    });
>      return override;

This doesn't seem like a useful change. The reference counts on the strings aren’t thread safe either. Same might apply for some of the other examples in this patch, but this one is clear cut case. Unless I am missing something?
Comment 9 Yusuke Suzuki 2020-08-15 13:11:02 PDT 
Comment on attachment 406660 [details]
Patch

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

>> Source/WTF/wtf/Language.cpp:87
>>      return override;
> 
> This doesn't seem like a useful change. The reference counts on the strings aren’t thread safe either. Same might apply for some of the other examples in this patch, but this one is clear cut case. Unless I am missing something?

The goal of this patch is fixing <rdar://problem/66774266> (RunLoop). So the other part is mechanical change.
I changed all `NeverDestroyed<ComplexData>` code in WTF mechanically to prevent the above type of issues.
If some of them are proven that this is not thread-related, I think we can change it to MainThreadNeverDestroyed<> etc.
Comment 10 Yusuke Suzuki 2020-08-15 21:40:51 PDT 
Comment on attachment 406660 [details]
Patch

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

>>> Source/WTF/wtf/Language.cpp:87
>>>      return override;
>> 
>> This doesn't seem like a useful change. The reference counts on the strings aren’t thread safe either. Same might apply for some of the other examples in this patch, but this one is clear cut case. Unless I am missing something?
> 
> The goal of this patch is fixing <rdar://problem/66774266> (RunLoop). So the other part is mechanical change.
> I changed all `NeverDestroyed<ComplexData>` code in WTF mechanically to prevent the above type of issues.
> If some of them are proven that this is not thread-related, I think we can change it to MainThreadNeverDestroyed<> etc.

Filed in https://bugs.webkit.org/show_bug.cgi?id=215546