Closed Bug 673176 Opened 8 years ago Closed 8 years ago

Separate image decoding telemetry by decoder


(Core :: ImageLib, defect)

Not set





(Reporter: justin.lebar+bug, Assigned: jrmuizel)




(1 file)

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.
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.
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.
You currently can't tell what type of decoder you're using, FWIW, so #2 is a non-starter.
Attachment #558548 - Flags: review?(justin.lebar+bug)
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.
Attachment #558548 - Flags: review?(justin.lebar+bug) → review+
Assignee: nobody → jmuizelaar
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.