1484349 - Optimize HasOwnProperty/GetProperty lookup for cloning

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Tom S [:evilpie]]

GetOwnPropertyPure should work most of the time, unless we have wrapped object.

Comment 1

[Tom S [:evilpie]]

Attachment: Optimize HasOwnProperty/GetProperty lookup for cloning. r?

Comment 2

[Tom S [:evilpie]]

Attachment: Support unboxed objects in GetPropertyPure

I've actually written a similar patch like this a few times before, but now I finally have a good reason to land it. With this patch we only take the non-GetOwnPropertyPure fallback path for wrapped objects during browser startup.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 9002107 [details] [diff] [review]
Optimize HasOwnProperty/GetProperty lookup for cloning. r?

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

::: js/src/vm/StructuredClone.cpp
@@ +1911,5 @@
>                  if (!startWrite(key))
>                      return false;
>              } else {
> +                // This relies on the way JSStructuredCloneWriter::traverseObject
> +                // converts JSIDs to Value.

Nit: no {} below.

We have to do this Value <-> jsid dance because of Map/Set/SavedFrame entries right? I think it would be nice to split "entries" in valueEntries (Vector of Values) and idEntries (Vector of jsids), or something. Or mapSetEntries/objectEntries. Then we can just use jsids everywhere for objects.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 9002112 [details] [diff] [review]
Support unboxed objects in GetPropertyPure

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

Nice

Comment 5

[Tom S [:evilpie]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> Comment on attachment 9002107 [details] [diff] [review]
> We have to do this Value <-> jsid dance because of Map/Set/SavedFrame
> entries right? I think it would be nice to split "entries" in valueEntries
> (Vector of Values) and idEntries (Vector of jsids), or something. Or
> mapSetEntries/objectEntries. Then we can just use jsids everywhere for
> objects.
Great idea.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2195ec4e254
Optimize HasOwnProperty/GetProperty lookup for cloning. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/8047901cef0e
Add unboxed object support GetProperyPure. r=jandem

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d2195ec4e254
https://hg.mozilla.org/mozilla-central/rev/8047901cef0e