339495 - 15% Tp regression on pawn, 4% on maya on 22 May 2006

Status: RESOLVED WONTFIX

(Core :: Widget: Cocoa, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

pawn and maya both took hits in Tp on 22 May sometime between 11am and noon-ish; pawn appeared to jump 15% (!) and maya 4% (pawn from a fairly-constant 952ms to a fairly-constant 1099ms, and maya from 305ms to 318ms).

pawn's last good build began at ca. 09:40 and the first bad build began ca. 11:40; maya's last good build began at ca. 11:06 and the first bad build began ca. 14:10.  Between the start of maya's last good build and the start of pawn's first bad one is http://tinyurl.com/hvbrw which looks like mento's checkin for bug 338357.

(The red build on pawn a couple of cycles later fubared the graph server for most of this week; it just today started actually loading graphs when requesting 7 days worth of data :-( thus the belated catch).

Seems to be Camino-only; planetoid was recovering from its own missing builds but stayed within an apparent range; atlantia, of course, doesn't run Tp :/

Comment 1

[Mark Mentovai]

338357 was Cocoa-only, and we don't have any Cocoa tinderboxes other than Camino, so it wouldn't have showed up anywhere else if that's the cause.

I guess I could go back to adding and removing things from the NSView hierarchy with deferred additions/removals?  I'm a little wary of doing it that way unless we know exactly what conditions cause the breakage and what conditons work.

Comment 2

[Mark Mentovai]

I just did some tests in cocoafox with a few alternate approaches and actually seem to get the best perf with the code that's currently in the tree.

Comment 3

[Mike Pinkerton (not reading bugmail)]

yay, so embedding is just SOL?

Comment 4

[Mark Mentovai]

Dunno yet, I want to do the same tests with Camino.

Comment 5

[Mark Mentovai]

Did a few new tests.  Now I see that the proposed changes do improve performance.  The weird numbers I saw yesterday might have been the result of doing perf testing on debug builds.  (Yeah, I know it's evil, but it's what I had available at the time.)

Carbonfox baseline:  809
Cocoafox current:   1039 (28% suck)
Cocoafox backout:    888 (15% better than current, but unstable)
Cocoafox alt. fix:   867 (17% better than current, may flicker?)
                         (still 7% suck relative to carbonfox)

Camino current:     1795
Camino backout:     1657 (8% better than current, but unstable)
Camino alt. fix:    1546 (14% better than current, may flicker?)

"Backout" is a backout of bug 338357, "alternate fix" uses the delayed-perform patch from that bug.

The Camino suck compared to Firefox is mostly because of the event model used for embedding apps, I think.

Comment 6

[Mark Mentovai]

Attachment: Patch used for "backout"

Comment 7

[Mark Mentovai]

Attachment: Patch used for "alternate fix"

Comment 8

[Mark Mentovai]

Attachment: Use delayed-perform to handle view show/hide

This is a clean version of "alternate fix," which fixes bug 338357 by delaying the NSView tree changes.  The problem is that when it's used with Camino, it causes flicker-to-white when a new page is loaded.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #8)
> The problem is that when it's used with Camino, it
> causes flicker-to-white when a new page is loaded.
 
Is that a different flicker-to-white than bug 313811?

Comment 10

[Stuart Parmenter]

Any idea what is going on with fixing this regression?

Comment 11

[Josh Aas]

At this point it is pretty buried. I think we shouldn't have been doing what we were doing before, so fixing this perf regression without a functional regression probably isn't going to happen. I remember spending a while looking into this with Mento and we didn't find a way to do it without bad behavior.

Comment 12

[David Zbarsky (:dzbarsky)]

So should this be closed?