Last Comment Bug 673176 - Separate image decoding telemetry by decoder
: Separate image decoding telemetry by decoder
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla9
Assigned To: Jeff Muizelaar [:jrmuizel]
: Milan Sreckovic [:milan]
Depends on: 672207
  Show dependency treegraph
Reported: 2011-07-21 11:34 PDT by Justin Lebar (not reading bugmail)
Modified: 2011-09-23 04:56 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Recording decoding speed for jpeg,png and gif (7.54 KB, patch)
2011-09-06 12:07 PDT, Jeff Muizelaar [:jrmuizel]
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2011-07-21 11:34:12 PDT
Bug 672207 adds image decoding telemetry, but it doesn't distinguish between how long different decoders take.

This is information is important if we want to set a different decode chunk size for different decoders.
Comment 1 Jeff Muizelaar [:jrmuizel] 2011-07-21 12:36:33 PDT
I've thought of a couple of different ways of doing this and haven't really decided on one yet:

1. have a virtual function on the decoder that returns a telemetry id
2. lookup a telemetry id based on the decoder type which I think we can get somehow
3. make the decoders responsible for recording their own telemetry.

Considerations: we probably care about telemetry for some decoders more than others. I'm not sure how much code duplication would happen if we made each decoder responsible for its own telemetry.
Comment 2 Joe Drew (not getting mail) 2011-09-06 09:10:26 PDT
Virtual function on the decoder is best IMO. We can return a sentinel value if we don't want to track a particular decoder's time.
Comment 3 Joe Drew (not getting mail) 2011-09-06 09:12:41 PDT
You currently can't tell what type of decoder you're using, FWIW, so #2 is a non-starter.
Comment 4 Jeff Muizelaar [:jrmuizel] 2011-09-06 12:07:55 PDT
Created attachment 558548 [details] [diff] [review]
Recording decoding speed for jpeg,png and gif
Comment 5 Justin Lebar (not reading bugmail) 2011-09-06 12:13:42 PDT
Comment on attachment 558548 [details] [diff] [review]
Recording decoding speed for jpeg,png and gif

> +    Telemetry::ID id = image->mDecoder->SpeedHistogram();
> +    if (id < Telemetry::HistogramCount) {
> +        Telemetry::Accumulate(image->mDecoder->SpeedHistogram(), PRInt32((image->mBytesDecoded/1024.0)/mDecodeTime.ToSeconds()));
> +    }

r=me, but please add a comment explaining why you have to check that id is valid, and please add some linebreaks to the Accumulate call.
Comment 6 Ed Morley [:emorley] 2011-09-23 04:56:57 PDT

Note You need to log in before you can comment on or make changes to this bug.