Bug 224171

Summary: [WTF] Introduce FixedVector and use it for FixedOperands
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, benjamin, cdumez, cmarcelo, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, msaboff, ryuan.choi, saam, sergio, simon.fraser, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
mark.lam: review+, ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch ews-feeder: commit-queue-

Description Yusuke Suzuki 2021-04-03 22:15:56 PDT
[WTF] Introduce FixedVector and use it for FixedOperands
Comment 1 Yusuke Suzuki 2021-04-03 22:19:13 PDT
Created attachment 425126 [details]
Patch
Comment 2 Yusuke Suzuki 2021-04-03 23:07:28 PDT
Created attachment 425127 [details]
Patch
Comment 3 Mark Lam 2021-04-05 09:44:10 PDT
Comment on attachment 425127 [details]
Patch

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

r=me

> Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:90
>      using RefCountedArray = WTF::RefCountedArray<int>;

I think we can remove this now.

> Source/JavaScriptCore/runtime/CachedTypes.cpp:625
> +    void encode(Encoder& encoder, const FixedVector<SourceType<T>>& vector)

It's a pity that we're copy-pasting this code 3 times.  Can we use just one template, and if needed, std::enable_if the container type to be one of the 3 acceptable types?

> Source/JavaScriptCore/runtime/CachedTypes.cpp:658
> +    void decode(Decoder& decoder, FixedVector<SourceType<T>>& vector, Args... args) const

Ditto.
Comment 4 Yusuke Suzuki 2021-04-05 11:33:29 PDT
Comment on attachment 425127 [details]
Patch

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

>> Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:90
>>      using RefCountedArray = WTF::RefCountedArray<int>;
> 
> I think we can remove this now.

Unfortunately, LLInt still uses RefCountedArray::m_data in

loadp CodeBlock::m_argumentValueProfiles + FixedVector::m_storage + RefCountedArray::m_data[t1], t3

>> Source/JavaScriptCore/runtime/CachedTypes.cpp:625
>> +    void encode(Encoder& encoder, const FixedVector<SourceType<T>>& vector)
> 
> It's a pity that we're copy-pasting this code 3 times.  Can we use just one template, and if needed, std::enable_if the container type to be one of the 3 acceptable types?

Changed them to use templatized ones :)

>> Source/JavaScriptCore/runtime/CachedTypes.cpp:658
>> +    void decode(Decoder& decoder, FixedVector<SourceType<T>>& vector, Args... args) const
> 
> Ditto.

Fixed.
Comment 5 Yusuke Suzuki 2021-04-05 12:25:03 PDT
Created attachment 425186 [details]
Patch
Comment 6 Yusuke Suzuki 2021-04-05 21:40:22 PDT
Created attachment 425239 [details]
Patch
Comment 7 Yusuke Suzuki 2021-04-06 07:56:30 PDT
Created attachment 425278 [details]
Patch
Comment 8 Yusuke Suzuki 2021-04-06 12:44:27 PDT
Existing flaky crashes.
Comment 9 Yusuke Suzuki 2021-04-06 12:45:55 PDT
mac-debug-wk1 is getting stuck.
Comment 10 Yusuke Suzuki 2021-04-06 12:47:57 PDT
Committed r275542 (236198@main): <https://commits.webkit.org/236198@main>
Comment 11 Radar WebKit Bug Importer 2021-04-06 12:48:17 PDT
<rdar://problem/76279966>