Closed Bug 597338 Opened 10 years ago Closed 10 years ago

Performance Issue, hovering Bookmark menuitem highlight is lagged

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b7
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: alice0775, Assigned: adw)

References

Details

(Keywords: perf, regression)

Attachments

(2 files, 2 obsolete files)

Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre ID:20100916041016

In cace of Long Bookmarks list,
hovering Bookmarks/history menuitem highlight is lagged.

This problem is more remarkable in Windows 7 Classic and/or old machine.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Make 100 boookmarks in a folder 
3. Open the folder  (or "Latest Headlines" on Bookmarks Toolbar).
4. Hover and move mouse pointer on menu item up and down

Actual Results:
 highlight is lagged

Expected Results:
 The highlight should follow the movement of the mouse immediately.


Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/c02f84806738
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100915 Firefox/4.0b7pre ID:20100915032449
Fails(lagged):
http://hg.mozilla.org/mozilla-central/rev/0caec4ddff74
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100915 Firefox/4.0b7pre ID:20100915061144
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c02f84806738&tochange=0caec4ddff74
blocking2.0: --- → ?
Confirmed on Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre
Currently, when you mouseover a link while no other hover link is visible in the location bar, there is a small delay before the link is shown.  One solution would be to always delay before showing a link, even if a hover link is already visible.  Net effect would be that a link is shown only when you deliberately stop on it, which might be nice anyway.
OS: Windows 7 → All
Hardware: x86 → All
Attached patch patch (obsolete) — Splinter Review
This builds on patch 3 in bug 596678 and does what comment 3 says.  I think it works pretty well at the slight cost of not updating the over-link instantly when it's already visible.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Attached patch patch 1.1 (obsolete) — Splinter Review
Forgot to delete _overLinkDelayTimer in one spot.  Again, this builds on patch 3 in bug 596678.
Attachment #476474 - Attachment is obsolete: true
Attachment #476487 - Flags: review?(dao)
Attachment #476487 - Flags: review?(dao) → review+
No code changes, updates the test to be compatible with new patch in bug 597930.
Attachment #476487 - Attachment is obsolete: true
Attachment #476945 - Flags: review?(dao)
Here's an alternate approach that doesn't get the over-link-box computed style on every call.

I ran some numbers on this patch and the previous patch, which gets the computed style on every call.  In 1,000 calls to setOverLink("http://example.com/") and setOverLink("") back-to-back, which simulates continuously mousing over a bookmarks menu, this patch took an average of 797ms, the previous patch 863ms.  For 10,000 calls, this patch 7.8s, previous patch 8.4s.

That's over 7% improvement, at the code-cost of caching whether the over-link is in transition (again).  This time I think I'm safe, since I check opacity before setting _overLinkTransitioning (after the timers fire).
Attachment #476978 - Flags: review?(dao)
Comment on attachment 476978 [details] [diff] [review]
patch: don't get computed style on each call

I don't think the Number() conversions are needed.
Attachment #476978 - Flags: review?(dao) → review+
Blocking for perception of performance impact.
blocking2.0: ? → betaN+
Attachment #476945 - Flags: review?(dao)
Whiteboard: [can land]
Target Milestone: Firefox 4.0 → ---
I filed this earlier https://bugzilla.mozilla.org/show_bug.cgi?id=594706, do I get a cookie?
Duplicate of this bug: 594706
(In reply to comment #10)
> I filed this earlier https://bugzilla.mozilla.org/show_bug.cgi?id=594706, do I
> get a cookie?

The thing that caused this bug landed after you filed your bug.  I have taken the cookie I had set aside for you and eaten it.
Hmm, this is weird. How come nobody noticed that this issue also existed in beta6?
http://hg.mozilla.org/mozilla-central/rev/6dbe4fca6137
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → Firefox 4.0b8
Had to back out because I can't land a bug without causing orange.

http://hg.mozilla.org/mozilla-central/rev/9c4a8b86ac0d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 4.0b8 → ---
Blocks: 600635
I landed again last night with a couple of small changes and haven't caused any orange in nearly 24 hours on the same types of boxes I oranged before, so calling this fixed.

If you still notice a regression with today's nightly (20101013) or later, please reopen.

http://hg.mozilla.org/mozilla-central/rev/ee82632da4df
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Thanks Kyle.

This sounded bad until I looked at the graph.  Earlier in the day before this bug landed and the day after this bug landed, the graph often spikes up to the same level that this regression is at.  The stddev for the 30 runs before mine is 494761.141.  Several changesets before mine there's even a small plateau.  It looks like I got unlucky and my changeset is at the start of a larger plateau.  But several changesets later that plateau ends.

I can't figure out how the get the graph to go back or forward.  I click the links and the bottom graph disappears.  I wanted to check more than one day before and after.  Anyway, based on a two-day window, it doesn't look like this changeset caused anything.
Bug 605680 will help here too.
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
You need to log in before you can comment on or make changes to this bug.