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

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Justin Lebar (not reading bugmail)]

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

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 2

[Justin Lebar (not reading bugmail)]

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

[Daniel C]

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.

Comment 4

[Justin Lebar (not reading bugmail)]

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

I'll fix it, thanks.

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch v2 (kilopixels/s, not kibipixel/s)

Comment 6

[Justin Lebar (not reading bugmail)]

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 7

[Justin Lebar (not reading bugmail)]

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.