1733075 - Optimize property enumeration more

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

On Google Sheets we spend a fair amount of time under Snapshot, called for for-in loops. We can optimize this better: in most cases the proto chain doesn't have any enumerable properties, so in that case we don't need to check for duplicates and iterate over all the prototype properties.

This also gets rid of some malloc lock contention for the duplicate-properties HashSet.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1733075 part 1 - Add HasEnumerable ObjectFlag. r?iain!

This gives us the ability to check whether an object has only non-enumerable
properties in constant time. Prototype objects typically don't have enumerable
properties.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1733075 part 2 - Optimize enumeration with no enumerable properties on the proto chain better. r?iain!

Add a fast path to better optimize the common case where the proto chain has no
enumerable properties. The advantages of this are:

• We no longer have to iterate over all the properties on the proto chain.
• We don't need to check for duplicates (this involves a relatively slow HashSet and includes non-enumerable properties too).

ProtoMayHaveEnumerableProperties returns false (meaning the fast path works) in
97-100% of cases on typical web workloads.

Depends on D127054

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1733075 part 3 - Optimize property enumeration in EnumerateNativeProperties. r?iain!

If we only care about enumerable properties and the object doesn't have any, we don't need
to iterate its properties.

We can't use this optimization in the CheckForDuplicates case, but fortunately the
optimization in the previous patch made that a lot less common.

Depends on D127055

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1733075 part 4 - Add tests for some for-in edge cases. r?iain!

At least some of these were not covered by other jit-tests.

Depends on D127056

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1733075 part 5 - Use NativeObject::hasEnumerableProperty in a few more places. r?anba!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b64b97a7733b
part 1 - Add HasEnumerable ObjectFlag. r=iain
https://hg.mozilla.org/integration/autoland/rev/474926a7192e
part 2 - Optimize enumeration with no enumerable properties on the proto chain better. r=iain
https://hg.mozilla.org/integration/autoland/rev/46ca817b5529
part 3 - Optimize property enumeration in EnumerateNativeProperties. r=iain
https://hg.mozilla.org/integration/autoland/rev/901076fcee17
part 4 - Add tests for some for-in edge cases. r=iain
https://hg.mozilla.org/integration/autoland/rev/5538d134615c
part 5 - Use NativeObject::hasEnumerableProperty in a few more places. r=anba

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b64b97a7733b
https://hg.mozilla.org/mozilla-central/rev/474926a7192e
https://hg.mozilla.org/mozilla-central/rev/46ca817b5529
https://hg.mozilla.org/mozilla-central/rev/901076fcee17
https://hg.mozilla.org/mozilla-central/rev/5538d134615c