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

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
7 years ago
7 years ago

People

(Reporter: justin.lebar+bug, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
See bug 715919 comment 17; uncompressed bytes / sec (or megapixels / sec) is a much better indicator of decoder speed than compressed bytes / sec.
(Reporter)

Comment 1

7 years ago
Created attachment 590323 [details] [diff] [review]
Patch v1
Attachment #590323 - Flags: review?(jmuizelaar)
(Reporter)

Comment 2

7 years ago
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.
(Reporter)

Comment 4

7 years ago
What, you don't like kibipixels?  :-p

I'll fix it, thanks.
(Reporter)

Comment 5

7 years ago
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)
(Reporter)

Comment 6

7 years ago
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."
(Reporter)

Comment 7

7 years ago
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-
(Reporter)

Updated

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