1484370 - Optimize native property iteration for structured cloning

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Tom S [:evilpie]]

Attachment: WIP

Reusing the ideas from Object.assign and various Object.values methods we can hand write an optimized iteration function that reduces intermediate allocations.

With all the patches including this one applied a simple array heavy serialization benchmark that I have been using improves from 350ms to 240ms. Looking at a profile I don't immediately see anything stand out anymore except for probably unavoidable memory allocation.

Comment 1

[Tom S [:evilpie]]

Attachment: Optimize native property iteration for structured cloning. r?

Comment 2

[Tom S [:evilpie]]

Comment on attachment 9002253 [details] [diff] [review]
Optimize native property iteration for structured cloning. r?

I have the feeling test coverage for structured cloning is not all that great on the SpiderMonkey side. However considering how much Firefox uses it, this try push seems to be good enough: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a51e5b94a1a654a8147d12128d401532c4d8cda0 (Only the translation stuff is a bit worrying, because it appears a few times).

In theory we don't even need to check for TypedArrayObjects etc. because we should only end up with PlainObject and ArrayObject here. Maybe I should just assert that.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 9002253 [details] [diff] [review]
Optimize native property iteration for structured cloning. r?

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

Nice speedup.

::: js/src/jit-test/tests/structured-clone/roundtrip.js
@@ +15,5 @@
> +    {0: 0, 1: 0, 1000000: 0, 1000001: 1},
> +
> +    [],
> +    [0, 1, 2],
> +    [0, 15, 16],

Add some with holes:

[0, , , 1, 2],
[, 1],
[0,,],
[,,],

::: js/src/vm/StructuredClone.cpp
@@ +1387,5 @@
> +    HandleNativeObject nobj = obj.as<NativeObject>();
> +    if (nobj->isIndexed() ||
> +        nobj->is<TypedArrayObject>() ||
> +        nobj->getClass()->getNewEnumerate() ||
> +        nobj->getClass()->getEnumerate())

At some point we should think about abstracting this. Maybe some kind of FastNativePropertyIterator template, but this is fine for now.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/075bfbeee249
Optimize native property iteration for structured cloning. r=jandem

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/075bfbeee249