Closed
Bug 597338
Opened 14 years ago
Closed 14 years ago
Performance Issue, hovering Bookmark menuitem highlight is lagged
Categories
(Firefox :: Address Bar, defect)
Firefox
Address Bar
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)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
blocking2.0: --- → ?
Reporter | ||
Comment 1•14 years ago
|
||
screen shot
http://www.youtube.com/watch?v=YWSXMV2-yeI
Confirmed on Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre
Assignee | ||
Comment 3•14 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•14 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Assignee | ||
Comment 4•14 years ago
|
||
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•14 years ago
|
||
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)
Updated•14 years ago
|
Attachment #476487 -
Flags: review?(dao) → review+
Assignee | ||
Comment 6•14 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•14 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 8•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #476945 -
Flags: review?(dao)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [can land]
Target Milestone: Firefox 4.0 → ---
Comment 10•14 years ago
|
||
I filed this earlier https://bugzilla.mozilla.org/show_bug.cgi?id=594706, do I get a cookie?
Assignee | ||
Comment 12•14 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•14 years ago
|
||
Hmm, this is weird. How come nobody noticed that this issue also existed in beta6?
Assignee | ||
Comment 14•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → Firefox 4.0b8
Assignee | ||
Comment 15•14 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 | ||
Comment 16•14 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
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
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.
Assignee | ||
Comment 18•14 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•14 years ago
|
||
Bug 605680 will help here too.
Updated•14 years ago
|
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
You need to log in
before you can comment on or make changes to this bug.
Description
•