1633217 - Object iteration is slower than the competition (3-4 times)

Status: UNCONFIRMED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Pierre-Yves Gérardy]

Attachment: iter-objects.js

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Iterating over objects in Firefox with for (let k in o) if (hasOwnProperty.call(x, k) is 3-4 times slower than in Chrome and Safari.

Actual results:

Timings:

Firefox Dev Ed 76.0b8:

{
"Object.keys(x).forEach(k => (result = x[k]))": {
"median": 431.3841133162518
},
"for (const k in x) if (hasOwn.call(x, k)) result = x[k]": {
"median": 334.95603961457385
},
"for (const k in x) if (x.hasOwnProperty(k)) result = x[k]": {
"median": 336.38636191471676
},
"for (const [k, v] of Object.entries(x)) result = v": {
"median": 731.6653770097341
}
}

Chrome 81.0.4044.113
{
"Object.keys(x).forEach(k => (result = x[k]))": {
"median": 168.31242426994694
},
"for (const k in x) if (hasOwn.call(x, k)) result = x[k]": {
"median": 94.88049556957459
},
"for (const k in x) if (x.hasOwnProperty(k)) result = x[k]": {
"median": 142.21746467214362
},
"for (const [k, v] of Object.entries(x)) result = v": {
"median": 187.50137985464804
}
}

Safari 13.1:
{
"Object.keys(x).forEach(k => (result = x[k]))": {
"median": 118.22721403792605
},
"for (const k in x) if (hasOwn.call(x, k)) result = x[k]": {
"median": 74.74650624520181
},
"for (const k in x) if (x.hasOwnProperty(k)) result = x[k]": {
"median": 129.76870497910724
},
"for (const [k, v] of Object.entries(x)) result = v": {
"median": 574.4314192711104
}
}

Expected results:

Times in the same ballpark.

Comment 1

[Nicolas B. Pierron [:nbp]]

Jan, can you look at this issue?

I presume this might be 3 different issues, but all related to our storage of the order of object properties. Today we have optimization for property accesses, but I do not think we have optimizations for the key ordering apart walking the shape tree everytime?

Comment 2

[André Bargull [:anba]]

V8 caches Object.keys for objects with few properties. This noticeable when running the test case with M = 19 and then comparing it to M = 20.

Comment 3

[Kannan Vijayan [:djvj]]

So, this is a problem that I talked about with Bas a while back (along with other ones), when we were addressing slowdowns in gdocs and got into looking at object representation in general.

It should be possible to cache (on a shape) the array of property names in the shape lineage up to that shape. The cached ordered-list-of-names vector can be left lazily uninitialized until a particular shape participates as a leaf-shape in a for-each listing.

All of this would imply an extra field on Shapes, which is a bit difficult right now given the memory usage implications. IMHO there should be a lot more leeway to consider this once we buy ourselves some memory breathing room by eliminating ObjectGroups (part of what WarpBuilder will entail). We will then have a lot more flexiblity in taking some of that free memory up for caches like these.

Just my 2 cents.

Comment 4

[Jan de Mooij [:jandem]]

Clearing NI per comment 2 and 3.

We have some known perf cliffs in this area (bug 1502905) and as Kannan said it would be nice to do better in the future with object model changes. However 3-4x on a micro-benchmark like this isn't too horrible of a perf issue that this needs urgent fixing I think.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:sdetar, could you have a look please?

For more information, please visit auto_nag documentation.