Closed Bug 1399533 Opened 5 years ago Closed 5 years ago
Remove duplicate checks in For
While working on bug 1376572, I noticed that there are duplicate checks which can be removed.
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)
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+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/054384780485 Remove duplicate checks in ForOfPIC::Chain. r=djvj
You need to log in before you can comment on or make changes to this bug.