Closed Bug 715240 Opened 8 years ago Closed 8 years ago

DECODE_ON_DRAW_LATENCY is wrong

Categories

(Core :: ImageLib, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Details

Attachments

(1 file)

No description provided.
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+
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/a5fa5e73d901
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.