Imagelib decoder speed telemetry should be uncompressed bytes / sec, not compressed bytes / sec




7 years ago
7 years ago


(Reporter: justin.lebar+bug, Unassigned)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

See bug 715919 comment 17; uncompressed bytes / sec (or megapixels / sec) is a much better indicator of decoder speed than compressed bytes / sec.
Created attachment 590323 [details] [diff] [review]
Patch v1
Attachment #590323 - Flags: review?(jmuizelaar)
I changed the name of the histograms so they don't get merged in the telemetry dashboard.

We could do KB/s instead of kilopixels/s, since the former is a more natural unit.  But since the *most* natural unit is MP/s, but that doesn't give us enough resolution, I used KP/s.

Comment 3

7 years ago
Comment on attachment 590323 [details] [diff] [review]
Patch v1

Review of attachment 590323 [details] [diff] [review]:

::: image/src/RasterImage.cpp
@@ +2863,5 @@
>          if (id < Telemetry::HistogramCount) {
> +            MOZ_ASSERT(image->mHasSize);
> +            // Report the decode rate in kilopixels per second.
> +            PRInt32 decodeRate = PRInt32(image->mSize.width * image->mSize.height /
> +                                         (1024.0 * mDecodeTime.ToSeconds()));

A kilopixel is 1,000 pixels, not 1,024.
What, you don't like kibipixels?  :-p

I'll fix it, thanks.
Created attachment 590426 [details] [diff] [review]
Patch v2 (kilopixels/s, not kibipixel/s)
Attachment #590323 - Attachment is obsolete: true
Attachment #590323 - Flags: review?(jmuizelaar)
Attachment #590426 - Flags: review?(jmuizelaar)
Gah, now twice in two days I've wanted the speed as compressed bytes / sec.

Uncompressed bytes / sec is useful for "how fast is the decoder?"  But compressed bytes / sec is useful for "how many bytes should I shove at the decoder if I have Xms to burn."
Comment on attachment 590426 [details] [diff] [review]
Patch v2 (kilopixels/s, not kibipixel/s)

The current histograms are useful, because we often ask the question "how many bytes should I shove at the decoder if I have Xms to burn?".  So at the very least, we should leave the current histograms and add these.  But upon retrospection, I'm not sure how useful these histograms are to us.
Attachment #590426 - Flags: review?(jmuizelaar) → review-


7 years ago
Last Resolved: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.