Bug 185803 – [DFG] InById should be converted to MatchStructure

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

...

[Yusuke Suzuki]

Created attachment 340787 [details]
Patch

[Yusuke Suzuki]

Created attachment 340788 [details]
Patch

[Yusuke Suzuki]

Created attachment 340789 [details]
Patch

[Yusuke Suzuki]

Created attachment 340793 [details]
Patch

[Caio Lima]

Comment on attachment 340793 [details]
Patch

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

I think it is good add tests considering proxy cases and check if we are not breaking it. Also, Is there any kind of program we could add on microbenchmark to check performance?

> Source/JavaScriptCore/bytecode/InByIdStatus.cpp:170
> +            ComplexGetStatus complexGetStatus = ComplexGetStatus::computeFor(structure, access.conditionSet(), uid);

Maybe it is time to rename this class to "ComplexPropertyAccessStatus" or something related? Current name is not reflecting the real usage of it.

[Yusuke Suzuki]

Comment on attachment 340793 [details]
Patch

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

Sounds nice, I've added a micorbenchmarks.

>> Source/JavaScriptCore/bytecode/InByIdStatus.cpp:170
>> +            ComplexGetStatus complexGetStatus = ComplexGetStatus::computeFor(structure, access.conditionSet(), uid);
> 
> Maybe it is time to rename this class to "ComplexPropertyAccessStatus" or something related? Current name is not reflecting the real usage of it.

I would like to make it a separate patch since it involves many changes.

[Yusuke Suzuki]

Created attachment 341571 [details]
Patch

[Yusuke Suzuki]

Ping?

[Keith Miller]

Comment on attachment 341571 [details]
Patch

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

r=me.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6458
> +                        data->variants.append(matchVariant);

Nit: Not that it matters now but we should probably WTFMove(matchVariant)

[Yusuke Suzuki]

Comment on attachment 341571 [details]
Patch

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

Thank you!

>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6458
>> +                        data->variants.append(matchVariant);
> 
> Nit: Not that it matters now but we should probably WTFMove(matchVariant)

Nice, fixed.

[Yusuke Suzuki]

Committed r232400: <https://trac.webkit.org/changeset/232400>

[Radar WebKit Bug Importer]

<rdar://problem/40728977>