Optimize EndLimitedIterator::GetNext()

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
9 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
I noticed this when profiling Speedometer Preact tests.  The bounds check in ElementAt() under GetNext() from <https://searchfox.org/mozilla-central/rev/b95a1a95838cc6e85ea5d9b9e3ae39b4cd69d8ef/dom/base/nsFrameMessageManager.cpp#924> shows up in profiles.

The comment there and the assertions show that the intention was for HasMore() to do the bounds check and not for another one here, but after bug 1159244 landed we forgot about this case and ended up with extra bounds checking.  I have a patch to remove this extra cost.
Comment on attachment 8878865 [details] [diff] [review]
Remove double bounds checking from the next accessors of the iterator classes of nsTObserverArray

Review of attachment 8878865 [details] [diff] [review]:
-----------------------------------------------------------------

This shows up in profiles?  Ugh.
Attachment #8878865 - Flags: review?(nfroyd) → review+

Comment 3

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b573b3cb047
Remove double bounds checking from the next accessors of the iterator classes of nsTObserverArray; r=froydnj

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3b573b3cb047
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → ehsan
You need to log in before you can comment on or make changes to this bug.