Closed
Bug 1399533
Opened 7 years ago
Closed 7 years ago
Remove duplicate checks in ForOfPIC::Chain
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla58
People
(Reporter: anba, Assigned: anba)
Details
Attachments
(1 file)
5.56 KB,
patch
|
djvj
:
review+
|
Details | Diff | Splinter Review |
While working on bug 1376572, I noticed that there are duplicate checks which can be removed.
Assignee | ||
Comment 1•7 years ago
|
||
Changes: - js::ForOfPIC::Chain::isArrayOptimized(...) is only called from js::ForOfPIC::Chain::tryOptimizeArray(...) and tryOptimizeArray(...) already ensures that isArrayStateStillSane() returns true, so we don't actually need to call isArrayStateStillSane() again in isArrayOptimized(...). - The array's prototype was checked when a stub was found and when it wasn't found, we can simplify this to check the prototype once before searching for a matching stub. - When no stub was found, and the number of stubs exceeded its limit, the stub chain was erased even when the new array will never be added to the stub chain, because it contains an own @@iterator property. It seems better to me to only erase the chain when the new array will definitely be added to the stub chain. - ForOfPIC::Chain::getMatchingStub(...) returns the matched stub, but the returned stub was only used to test it against nullptr, so I renamed the method to hasMatchingStub(...) and changed the return type to |bool|. - hasMatchingStub(...) contains dynamic checks to ensure that the chain is initialized and not disabled, but since both conditions are already ensured in tryOptimizeArray(...), I've changed the tests into an assertion. - And I've changed the argument type in hasMatchingStub(...) from JSObject* to ArrayObject*, that way we can directly call ShapedObject::shape(), which saves us the type checks in JSObject::maybeShape().
Attachment #8907694 -
Flags: review?(kvijayan)
Updated•7 years ago
|
Priority: -- → P3
Comment 2•7 years ago
|
||
Comment on attachment 8907694 [details] [diff] [review] bug1399533.patch Review of attachment 8907694 [details] [diff] [review]: ----------------------------------------------------------------- Nice, looks good!
Attachment #8907694 -
Flags: review?(kvijayan) → review+
Assignee | ||
Comment 3•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=870329d65fc96d37befc13269ffca15d95693ef1
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/054384780485 Remove duplicate checks in ForOfPIC::Chain. r=djvj
Keywords: checkin-needed
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/054384780485
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•