Closed Bug 424675 Opened 16 years ago Closed 12 years ago

20% Twinopen regression on Vista

Categories

(Firefox :: General, defect, P2)

x86
Windows Vista
defect

Tracking

()

RESOLVED INCOMPLETE
Firefox 3

People

(Reporter: jimm, Unassigned)

Details

(Keywords: perf, regression)

Attachments

(5 files, 1 obsolete file)

This might have been files someplace else, but all I found in bugzilla was bug 421072 from the 4th. The date of this was around the 17 -> 18th of March. Range:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-03-17&maxdate=2008-03-18&cvsroot=%2Fcvsroot

graphs screenshot forthcoming
Attached image graph
Flags: blocking-firefox3?
Apparently this was caused by bug 422420.

Yeah, that's right. Bug 422420. Nobody knows why.
Assignee: nobody → mconnor
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Target Milestone: --- → Firefox 3
I'm going to try to use aqtime to track this down. While working on it I'll be posting any interesting stuff I find. Here's a simple run on xul.dll. GetShellFolderPath (ranks 7th) is interesting.
Jim, can you see if you can dig into this any better?  I've hit a bit of a wall.
Assignee: mconnor → jmathies
Yep, plan to, I was right in the middle of this but got sidetracked on some other stuff that came up.
A comparison of the two. Paint / font related calls seem off quite a bit. This is run of twinopen (10 windows) and it's running under aqtime so the total times for methods / functions are going to be exaggerated.
2nd set of runs, same settings.
I did a bunch of profiling runs over the day looking for numbers that point to the patch for Bug 422420. In the end I've come to conclusion that patch may not be it.
Jim, backing out the patch undid the regression on the vista tinderboxes, so we're confident this was the cause.  Are you seeing the difference outside of the profilere (just the raw time)?
I don't see a difference in time equal to that of tinderbox. I do see a much smaller difference (about ~15 msec) which is also in my profiling results which looks to be related to window event handling and painting. Honestly though with numbers this small it's often hard to reproduce them reliably. Attached is a profiling run with twinopen doing 100 windows with and without. The total accumulated differences are only 1-2 seconds in some paint routines, which is really small for a run of 100 windows.

If we're sure this is the culprit I'll look for ways to minimize the small differences here by fiddling with the patch. That would hopefully fix the bigger problem on tinderbox.  
Attached file 100 windows R1:with R2: without (obsolete) —
note that's just a small snip of the data but it's where I see the small changes I'm referring to. Most everything else matches up, or the numbers are too small to tell.
accidentally snipped out some of the "good stuff"
Attachment #315775 - Attachment is obsolete: true
I think we might be looking at this from the wrong angle. I don't think this is a performance hit we're taking by adding a button to the nav-bar, it's a performance gain we get when we add a home button to the PersonalToolbar. For example, without the patch applied, rotating through these test cases -

PersonalToolbar defaultset has home-button - lower twinopen numbers

PersonalToolbar defaultset has home-button / nav-bar defaultset has home-button - lower twinopen numbers

PersonalToolbar defaultset does not have home-button / nav-bar defaultset has home-button - higher twinopen numbers

PersonalToolbar defaultset does not have home-button / nav-bar defaultset does not have home-button - higher twinopen numbers

PersonalToolbar defaultset has home-button / nav-bar defaultset does not have home-button - lower twinopen numbers
Actually there's a bunch of code from the original patch nsBrowserGlue I think can now be removed. Some of this looks like the likely culprit.

https://bugzilla.mozilla.org/attachment.cgi?id=303388&action=diff
nope, most of that was removed. still looking...
We know what caused this, but the same regression didn't happen on XP, and its really unclear why this happened.  The current tooling hasn't yielded anything of value, and its time to ship, so we're going to keep an eye on this, but not continue to block.
Flags: wanted1.9.0.x+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Assignee: jmathies → nobody
Jim / Mike: Last update in 2008 - can this be closed as obsolete?
(In reply to Andre Klapper from comment #20)
> Jim / Mike: Last update in 2008 - can this be closed as obsolete?

Yes, this is ancient.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.