597338 - Performance Issue, hovering Bookmark menuitem highlight is lagged

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Alice0775 White]

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

Comment 1

[Alice0775 White]

screen shot
http://www.youtube.com/watch?v=YWSXMV2-yeI

Comment 2

[Andrea]

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

Comment 3

[Drew Willcoxon :adw]

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.

Comment 4

[Drew Willcoxon :adw]

Attachment: patch

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.

Comment 5

[Drew Willcoxon :adw]

Attachment: patch 1.1

Forgot to delete _overLinkDelayTimer in one spot.  Again, this builds on patch 3 in bug 596678.

Comment 6

[Drew Willcoxon :adw]

Attachment: patch 1.1 with test fix

No code changes, updates the test to be compatible with new patch in bug 597930.

Comment 7

[Drew Willcoxon :adw]

Attachment: patch: don't get computed style on each call

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).

Comment 8

[Dão Gottwald [: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.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Blocking for perception of performance impact.

Comment 10

[Timofei Shatrov]

I filed this earlier https://bugzilla.mozilla.org/show_bug.cgi?id=594706, do I get a cookie?

Comment 12

[Drew Willcoxon :adw]

(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

[Timofei Shatrov]

Hmm, this is weird. How come nobody noticed that this issue also existed in beta6?

Comment 14

[Drew Willcoxon :adw]

http://hg.mozilla.org/mozilla-central/rev/6dbe4fca6137

Comment 15

[Drew Willcoxon :adw]

Had to back out because I can't land a bug without causing orange.

http://hg.mozilla.org/mozilla-central/rev/9c4a8b86ac0d

Comment 16

[Drew Willcoxon :adw]

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

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/9f281275d565657c# says this may have caused substantial memory use regressions on OS X 10.5.

Comment 18

[Drew Willcoxon :adw]

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.

Comment 19

[Drew Willcoxon :adw]

Bug 605680 will help here too.