Closed
Bug 414186
Opened 16 years ago
Closed 16 years ago
slow loop code in nsICODecoder?
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla1.9beta4
People
(Reporter: Dolske, Assigned: Dolske)
Details
Attachments
(1 file)
1.21 KB,
patch
|
pavlov
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
Found this lurking in nsICODecoder::ProcessData() when poking at bug 413512... 224 while (aCount && mPos < mImageOffset) { // Skip to our offset 225 mPos++; aBuffer++; aCount--; 226 } The offset being skipped to can easily be many kilobytes from the current offset, which would mean this loop would be going through thousands of iterations. I dunno if the compiler is smart enough to optimize this down to what the basic arithmetic would do, but I'm always pessimistic about such magic. :-)
Assignee | ||
Comment 1•16 years ago
|
||
Obvious fix. http://browser.garage.maemo.org/ has one of the largest favicons I've noticed (31K), this section of code is responsible for incrementing mPos from 102 to 30574. :-)
Assignee | ||
Comment 2•16 years ago
|
||
I stumbled across some similarly slow-looking code in the GIF decoder: /* init the tables */ for (int i = 0; i < 4096; i++) mGIFStruct.suffix[i] = i; Out of curiosity, I measured it... In a optimized build that GIF code takes ~1 microsecond (2.4Ghz, Solaris). Measuring the ICO loop (this bug) is ~64 microseconds. So, not a noticeable perf change, but the patch makes stepping through the in a debugger less painful. :-) I should have disassembled this code to see if it's smartly optimized or just hauling ass in the cache... but meh.
Updated•16 years ago
|
Attachment #299533 -
Flags: review?(pavlov) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #299533 -
Flags: approval1.9?
Updated•16 years ago
|
Flags: blocking1.9+
Updated•16 years ago
|
Attachment #299533 -
Flags: approval1.9? → approval1.9+
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: blocking1.9+
Resolution: --- → FIXED
Assignee | ||
Comment 3•16 years ago
|
||
Checking in modules/libpr0n/decoders/bmp/nsICODecoder.cpp; new revision: 1.47; previous revision: 1.46 Removing perf keyword, since the measurements I did showed this wasn't an issue in optimized (ie, shipping) builds.
Keywords: perf
Assignee | ||
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9beta4
You need to log in
before you can comment on or make changes to this bug.
Description
•