More Object.hasOwnProperty optimizations

RESOLVED WONTFIX

Status

()

enhancement
RESOLVED WONTFIX
2 years ago
2 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks 3 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf-])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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
(Assignee)

Comment 2

2 years ago
This improves a simple benchmark from 61ms to 37ms.
Assignee: nobody → evilpies
(Assignee)

Comment 3

2 years ago
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.
(Assignee)

Updated

2 years ago
Depends on: 1357468
(Assignee)

Updated

2 years ago
Depends on: 654190
(Assignee)

Comment 5

2 years ago
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.

Updated

2 years ago
Whiteboard: [qf]
This pattern is common in many places including Speedometer.
Whiteboard: [qf] → [qf:p1]
(Assignee)

Comment 7

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

Updated

2 years ago
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?
(Assignee)

Comment 10

2 years ago
Yeah let's close this.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
(Assignee)

Updated

2 years ago
Depends on: 1404659
You need to log in before you can comment on or make changes to this bug.