Closed
Bug 1383362
Opened 7 years ago
Closed 7 years ago
Use for loop rather than forEach in Services.jsm
Categories
(Toolkit :: General, enhancement)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
This winds up being about twice as fast.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8889003 [details]
Bug 1383362: Use for-of loop rather than forEach() in Services.jsm.
https://reviewboard.mozilla.org/r/160042/#review165454
I saw this .forEach in profiles a few times too. Also true of the .forEach calls used for similar purposes in the root scopes nsBrowserGlue.js and browser.js
(In reply to Kris Maglione [:kmag] from comment #0)
> This winds up being about twice as fast.
Out of curiosity, how did you measure this?
The last time I tried to compare forEach, for..of and for (let i; i < array.length; ++i), benchmarking correctly ended up being non trivial, and I had to get help from JS engine folks to get correct answers... which were unfortunately much more complicated than I initially hoped for. See bug 1355874 comment 8.
Attachment #8889003 -
Flags: review?(florian) → review+
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8889003 [details]
Bug 1383362: Use for-of loop rather than forEach() in Services.jsm.
https://reviewboard.mozilla.org/r/160042/#review165454
I just measured the time it takes to load the Services.jsm root scope according to the profiler.
It's hard to really say whether for-of is faster than forEach in the general case, but it definitely is faster here, and has also been faster in other slow sections of code that I've changed in the past.
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6469a8fbaae429b58fc07d4a447e4c2e294328f7
Bug 1383362: Use for-of loop rather than forEach() in Services.jsm. r=florian
Comment 5•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•