DECODE_ON_DRAW_LATENCY is wrong

RESOLVED FIXED in mozilla12

Status

()

Core
ImageLib
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla12
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 585825 [details] [diff] [review]
Only start tracking the time when we have all of the source data
Attachment #585825 - Flags: review?(justin.lebar+bug)
It's wrong in two ways.

First, the check

>  if (!mDecoded) {
>      mDrawStartTime = TimeStamp::Now();
>  }

should be

>  if (!mDecoded && mHasSourceData)

because both predicates together mean "discarded".

Also, the IMAGE_DECODE_ON_DRAW_LATENCY histogram is not being frobbed often enough.  When I switch to a tab containing 45 large, discarded images, the histogram is frobbed only twice.
Also, is Telemetry::IMAGE_DECODE_TIME supposed to track the time it takes to re-decode an image?  It doesn't appear to at the moment.
> Also, the IMAGE_DECODE_ON_DRAW_LATENCY histogram is not being frobbed often enough.  When I switch 
> to a tab containing 45 large, discarded images, the histogram is frobbed only twice.

Oh.  The images aren't decoded until they appear onscreen.
Comment on attachment 585825 [details] [diff] [review]
Only start tracking the time when we have all of the source data

This looks right to me, but I'd like joe to have a look.
Attachment #585825 - Flags: review?(justin.lebar+bug)
Attachment #585825 - Flags: review?(joe)
Attachment #585825 - Flags: feedback+
Assignee: nobody → jmuizelaar
Comment on attachment 585825 [details] [diff] [review]
Only start tracking the time when we have all of the source data

Review of attachment 585825 [details] [diff] [review]:
-----------------------------------------------------------------

::: image/src/RasterImage.cpp
@@ +2578,5 @@
>      mFrameDecodeFlags = DECODE_FLAGS_DEFAULT;
>    }
>  
> +  // XXX: we use !mDecoded && mHasSourceData to mean
> +  // discarded. I'm not sure if this is true or not.

This is correct. Fix up the comment.
Attachment #585825 - Flags: review?(joe) → review+
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/a5fa5e73d901
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.