673176 - Separate image decoding telemetry by decoder

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Justin Lebar (not reading bugmail)]

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]]

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)]

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)]

You currently can't tell what type of decoder you're using, FWIW, so #2 is a non-starter.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Recording decoding speed for jpeg,png and gif

Comment 5

[Justin Lebar (not reading bugmail)]

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]]

https://hg.mozilla.org/mozilla-central/rev/a5273606afed