Closed Bug 1356315 Opened 3 years ago Closed 3 years ago

More Object.hasOwnProperty optimizations

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 3 open bugs)

Details

(Whiteboard: [qf-])

Attachments

(1 file)

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.
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
This improves a simple benchmark from 61ms to 37ms.
Assignee: nobody → evilpies
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.
Depends on: 1357468
Depends on: 654190
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.
Whiteboard: [qf]
This pattern is common in many places including Speedometer.
Whiteboard: [qf] → [qf:p1]
Fwiw, I didn't see any improvements on Speedometer with this patch. I think v8 removed this optimization after moving to Turbofan.
No longer blocks: TimeToFirstPaint_FB
Changing to qf- because of negative results reported by evilpie.
Whiteboard: [qf:p1] → [qf-]
Tom, should this be closed as wontfix, based on comment 7?
Yeah let's close this.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Depends on: 1404659
You need to log in before you can comment on or make changes to this bug.