Bug 215535

Summary:

Use std::call_once + LazyNeverDestroyed to initialize complex data structures

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

annulen, benjamin, cdumez, cmarcelo, darin, ews-watchlist, gyuyoung.kim, mark.lam, ryuan.choi, sergio, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	mark.lam: review+
Patch	none

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