1356315 - More Object.hasOwnProperty optimizations

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Tom S [:evilpie]]

There is still some stuff we should do:
1) Optimize dense element holes in CacheIR
2) More constant folding in Ion, e.g. something using the definite property analysis.
3) Use baseline IC data in Ion to inline?
4) Optimize the following pattern, found in react, ember, pdf.js etc.

for (var x in obj) {
  if (obj.hasOwnProperty(x))
    break;
}

We can just look at the iterator object to decide if x is an own property of obj.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

For what it's worth, the pattern in 4) is part of a style rule of eslint, a linter for JS commonly used in the wild, so it might be very useful to optimize in general: http://eslint.org/docs/rules/guard-for-in

Comment 2

[Tom S [:evilpie]]

Attachment: POC Optimize the for-in hasOwnProperty pattern

This improves a simple benchmark from 61ms to 37ms.

Comment 3

[Tom S [:evilpie]]

I thought we could get away with not shape-checking the object, because we would only call hasOwnProperty on ids that we iterated over. And deletions would be handled by the suppression mechanism. However it's possible define a property on the object during the iteration, which was previously only on the proto.

var o = {x: 1, __proto__: {y: 2}};
for (var id in o) {
	print(o.hasOwnProperty(id));
  o.y = 1;
}

This should print true, twice.

Comment 4

[Tom S [:evilpie]]

Ember seems to never hit fallback path \o/. But React does
1. Iterating over an array: https://github.com/WebKit/webkit/blob/master/PerformanceTests/Speedometer/resources/todomvc/labs/architecture-examples/react/bower_components/react/JSXTransformer.js#L5913
2. Shape change during iteration: https://github.com/WebKit/webkit/blob/master/PerformanceTests/Speedometer/resources/todomvc/labs/architecture-examples/react/bower_components/react/react.js#L7179

Comment 5

[Tom S [:evilpie]]

Try push for the for..in hasOwnProperty optimization looks quite good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=61e1b526a238ab4a4451ea1abad3a8af1a894fa6

I need to do some analysis on speedometer to figure out if this is actually a win.

Comment 6

[Naveed Ihsanullah [:naveed]]

This pattern is common in many places including Speedometer.

Comment 7

[Tom S [:evilpie]]

Fwiw, I didn't see any improvements on Speedometer with this patch. I think v8 removed this optimization after moving to Turbofan.

Comment 8

[Kannan Vijayan [:djvj]]

Changing to qf- because of negative results reported by evilpie.

Comment 9

[Kannan Vijayan [:djvj]]

Tom, should this be closed as wontfix, based on comment 7?

Comment 10

[Tom S [:evilpie]]

Yeah let's close this.