Last Comment Bug 922770 - Increase default threshold for "stable frame" in eideticker
: Increase default threshold for "stable frame" in eideticker
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: Eideticker (show other bugs)
: unspecified
: x86_64 Linux
-- normal (vote)
: ---
Assigned To: William Lachance (:wlach) (use needinfo!)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-01 11:53 PDT by William Lachance (:wlach) (use needinfo!)
Modified: 2013-10-16 14:52 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch (1.94 KB, patch)
2013-10-15 09:33 PDT, William Lachance (:wlach) (use needinfo!)
dave.hunt: review-
Details | Diff | Splinter Review
Remove accidental submodule updates (1.37 KB, patch)
2013-10-15 14:53 PDT, William Lachance (:wlach) (use needinfo!)
dave.hunt: review+
Details | Diff | Splinter Review

Description User image William Lachance (:wlach) (use needinfo!) 2013-10-01 11:53:16 PDT
In this capture: http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28fresh%20profile%29%20%282013-09-30%29&video=videos/video-1380613046.49.webm&framediff=framediffs/framediff-1380613056.1.json&actionlog=

It's pretty clear that it's done loading at around 2.7s, but there's a transition where the scrollbar disappears, which momentarily causes a 2580 pixel change, which exceeds the threshold, so we say it ends at around 3.25 seconds.

Let's increase the default threshold to 4096, as actual major changes in loading are likely to exceed that greatly (as the above capture shows).
Comment 1 User image Mark Finkle (:mfinkle) (use needinfo?) 2013-10-01 12:43:17 PDT
I looked at some of the pageload tests, to see if increasing the threshold would effect their "end times", but I saw pixel differences in the 7K range at the very end of pageload. Bumping the threshold to 4096 won't suddenly make pageload tests seem to run faster, which is a good thing.
Comment 2 User image Mark Finkle (:mfinkle) (use needinfo?) 2013-10-09 21:53:48 PDT
William - Any ETA for this change?
Comment 3 User image William Lachance (:wlach) (use needinfo!) 2013-10-15 09:33:38 PDT
Created attachment 817232 [details] [diff] [review]
0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch

Adjust framediff threshold default (implicitly this is an Android-only change).
Comment 4 User image William Lachance (:wlach) (use needinfo!) 2013-10-15 09:35:27 PDT
(In reply to Mark Finkle (:mfinkle) from comment #2)
> William - Any ETA for this change?

Sorry for the delay on such a simple change, was on PTO after the summit and things have generally been hectic.

I have applied to patch manually pending review, so new builds should be using the setting.
Comment 5 User image Dave Hunt (:davehunt) 2013-10-15 14:42:39 PDT
Comment on attachment 817232 [details] [diff] [review]
0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch

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

Are the submodule updated intentional?
Comment 6 User image William Lachance (:wlach) (use needinfo!) 2013-10-15 14:53:21 PDT
(In reply to Dave Hunt (:davehunt) from comment #5)
> Comment on attachment 817232 [details] [diff] [review]
> 0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch
> 
> Review of attachment 817232 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Are the submodule updated intentional?

No. I'll upload a new patch without this problem.
Comment 7 User image William Lachance (:wlach) (use needinfo!) 2013-10-15 14:53:55 PDT
Created attachment 817468 [details] [diff] [review]
Remove accidental submodule updates
Comment 8 User image Dave Hunt (:davehunt) 2013-10-16 02:59:03 PDT
Comment on attachment 817468 [details] [diff] [review]
Remove accidental submodule updates

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

Looks good.
Comment 9 User image William Lachance (:wlach) (use needinfo!) 2013-10-16 14:52:19 PDT
Pushed: https://github.com/mozilla/eideticker/commit/27c59525e585d4b33bfbb73fb521dca455e8e197

Note You need to log in before you can comment on or make changes to this bug.