Bug 185348 – [JSC] Object.assign for final objects should be faster

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Object.assign for final objects should be faster

[Yusuke Suzuki]

Created attachment 339637 [details]
WIP

[Yusuke Suzuki]

Created attachment 339638 [details]
WIP

[Yusuke Suzuki]

Created attachment 339639 [details]
WIP

[EWS Watchlist]

Comment on attachment 339639 [details]
WIP

Attachment 339639 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7575044

New failing tests:
http/tests/security/video-poster-cross-origin-crash2.html

[EWS Watchlist]

Created attachment 339645 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Yusuke Suzuki]

Created attachment 339666 [details]
Patch

[Yusuke Suzuki]

Created attachment 339668 [details]
Patch

[Yusuke Suzuki]

Created attachment 339671 [details]
Patch

[Saam Barati]

Comment on attachment 339671 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:316
> +            if (!source->staticPropertiesReified()) {

Why do this instead of just walk them? JSObject::getOwnPropertyNames walks these instead of reifying them

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:344
> +                structure->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {

Do you want to ignore private symbols here?

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:360
> +        targetCanPerformFastPut = false;

Make sure you have a test for exactly this if you don’t already

[Yusuke Suzuki]

Comment on attachment 339671 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:316
>> +            if (!source->staticPropertiesReified()) {
> 
> Why do this instead of just walk them? JSObject::getOwnPropertyNames walks these instead of reifying them

We would like to reflect static properties first before checking structure's flags since structure's flag would not reflect the properties defined by static properties.

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:344
>> +                structure->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {
> 
> Do you want to ignore private symbols here?

Nice catch! Fixed, and added the test for this.

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:360
>> +        targetCanPerformFastPut = false;
> 
> Make sure you have a test for exactly this if you don’t already

I've added the test.

[Yusuke Suzuki]

Created attachment 339943 [details]
Patch

[Yusuke Suzuki]

Ping?

[Saam Barati]

Comment on attachment 339943 [details]
Patch

r=me

[Yusuke Suzuki]

Committed r231687: <https://trac.webkit.org/changeset/231687>

[Radar WebKit Bug Importer]

<rdar://problem/40152352>

[Yusuke Suzuki]

https://perf.webkit.org/v3/#/charts?since=1525432412425&paneList=((19-906))
Yay