Performance Issue, hovering Bookmark menuitem highlight is lagged

RESOLVED FIXED in Firefox 4.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: alice0775, Assigned: adw)

Tracking

({perf, regression})

Trunk
Firefox 4.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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
(Reporter)

Updated

9 years ago
blocking2.0: --- → ?

Comment 2

9 years ago
Confirmed on Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre
(Assignee)

Comment 3

9 years ago
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.
(Assignee)

Updated

9 years ago
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Comment 4

9 years ago
Posted 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
(Assignee)

Comment 5

9 years ago
Posted 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+
(Assignee)

Comment 6

9 years ago
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)
(Assignee)

Comment 7

9 years ago
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)
(Assignee)

Updated

9 years ago
Whiteboard: [can land]
Target Milestone: Firefox 4.0 → ---

Comment 10

9 years ago
I filed this earlier https://bugzilla.mozilla.org/show_bug.cgi?id=594706, do I get a cookie?
Duplicate of this bug: 594706
(Assignee)

Comment 12

9 years ago
(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.

Comment 13

9 years ago
Hmm, this is weird. How come nobody noticed that this issue also existed in beta6?
(Assignee)

Comment 14

9 years ago
http://hg.mozilla.org/mozilla-central/rev/6dbe4fca6137
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → Firefox 4.0b8
(Assignee)

Comment 15

9 years ago
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 → ---
(Assignee)

Updated

9 years ago
Blocks: 600635
(Assignee)

Comment 16

9 years ago
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
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
(Assignee)

Comment 18

9 years ago
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.
(Assignee)

Comment 19

9 years ago
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.