Closed Bug 414186 Opened 16 years ago Closed 16 years ago

slow loop code in nsICODecoder?

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: Dolske, Assigned: Dolske)

Details

Attachments

(1 file)

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. :-)
Attached patch Patch v.1Splinter Review
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: nobody → dolske
Status: NEW → ASSIGNED
Attachment #299533 - Flags: review?(pavlov)
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.
Attachment #299533 - Flags: review?(pavlov) → review+
Keywords: perf
Attachment #299533 - Flags: approval1.9?
Flags: blocking1.9+
Attachment #299533 - Flags: approval1.9? → approval1.9+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: blocking1.9+
Resolution: --- → FIXED
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
Target Milestone: --- → mozilla1.9beta4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: