1411774 - Optimize Object.assign with unboxed objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tom S [:evilpie]]

Attachment: WIP

It's relatively like and it does happen on the web that code like Object.assign(obj, {a: 1, b: 3}) will turn the object literal into an unboxed object. This current always takes a slow path.

In my patch for this I am not sure if we need to copy the layout.properties(), because those might change during execution?

This makes unboxed objects as a source about as fast as native objects, maybe a bit slower.

Comment 1

[Tom S [:evilpie]]

> In my patch for this I am not sure if we need to copy the layout.properties(), because those might change during execution?
Brian can you answer this for me? Thanks

Comment 2

[Brian Hackett [Laid off!]]

(In reply to Tom Schuster [:evilpie] from comment #0)
> In my patch for this I am not sure if we need to copy the
> layout.properties(), because those might change during execution?

The properties in a group's unboxed layout do not change after construction.

Comment 3

[Tom S [:evilpie]]

Attachment: Optimize Object.assign from an unboxed object

Thanks Brian. I was actually wondering what keeps the UnboxedLayout alive, but I didn't phrase that well. During the iteration we might change from unboxed to native.

Comment 4

[Brian Hackett [Laid off!]]

(In reply to Tom Schuster [:evilpie] from comment #3)
> Created attachment 8923553 [details] [diff] [review]
> Optimize Object.assign from an unboxed object
> 
> Thanks Brian. I was actually wondering what keeps the UnboxedLayout alive,
> but I didn't phrase that well. During the iteration we might change from
> unboxed to native.

Well, the unboxed layout is owned by / stored in the object group which refers to it, and since that group is rooted in the patch things should be fine.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8923553 [details] [diff] [review]
Optimize Object.assign from an unboxed object

Review of attachment 8923553 [details] [diff] [review]:
-----------------------------------------------------------------

\o/ very nice to have this fixed!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cbf1345efd1
Optimize Object.assign with unboxed objects. r=jandem

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing spidermonkey non-unified at js/src/vm/UnboxedObject.h:263:33: inline function 'const js::UnboxedLayout& js::UnboxedPlainObject::layout() const' used but never defined:

https://hg.mozilla.org/integration/mozilla-inbound/rev/4154937d7b4e4d217f37cb6d06fd51e5012cbe84

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8cbf1345efd1e9fb1f6a729af5a115431fcd4c15&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=141112011&repo=mozilla-inbound
> js/src/vm/UnboxedObject.h:263:33: error: inline function 'const js::UnboxedLayout& js::UnboxedPlainObject::layout() const' used but never defined [-Werror]

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2aaa2d20e7aa
Optimize Object.assign with unboxed objects. r=jandem

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2aaa2d20e7aa