Bug 224171 - [WTF] Introduce FixedVector and use it for FixedOperands
Summary: [WTF] Introduce FixedVector and use it for FixedOperands
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-03 22:15 PDT by Yusuke Suzuki
Modified: 2021-04-06 13:54 PDT (History)
15 users (show)

See Also:

Attachments
Patch (52.37 KB, patch)
2021-04-03 22:19 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (52.51 KB, patch)
2021-04-03 23:07 PDT, Yusuke Suzuki 		mark.lam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (53.51 KB, patch)
2021-04-05 12:25 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (67.68 KB, patch)
2021-04-05 21:40 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (67.82 KB, patch)
2021-04-06 07:56 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (3) 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 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>