Closed Bug 880554 Opened 11 years ago Closed 11 years ago

Talos tresize chromez regression of 400% on UX 1019becd7a2a from OS X titlebar patches

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: MattN, Assigned: mstange)

References

()

Details

(Keywords: perf, Whiteboard: [Australis:M7])

Attachments

(1 file)

Attached image Screenshot of spike in tresize (deleted) —
See the URL for the graph. The theory is that this didn't affect m-c because it doesn't have the tabs in the titlebar. Pushlog: http://hg.mozilla.org/projects/ux/pushloghtml?changeset=1019becd7a2a
Including 10.7 and 10.8 in the graph: 10.6 – 46% regression 10.7 – 161% regression 10.8 – 411% regression
Summary: 46% Talos tresize chromez regression on UX 1019becd7a2a from OS X titlebar patches → Talos tresize chromez regression of 400% on UX 1019becd7a2a from OS X titlebar patches
The numbers went up because the old titlebar drawing caused faulty measurements. Now the measurement bug is fixed and the numbers have gone up to their usual level. In the graphs you can see that the UX numbers used to be below the Firefox (mozilla-central) numbers, and now they're at the same level. TResize measures the time that elapses between the start of a call to window.resize and the next MozAfterPaint event. With the old titlebar drawing, there was an additional MozAfterPaint event fired for drawing only the titlebar, before the window content was redrawn. So the test only measured titlebar drawing when it wanted to measure whole window drawing. Before the fix, this MozAfterPaint event was fired from the following callstack: DelayedFireDOMPaintEvent::Run() nsContentUtils::RemoveScriptBlocker() nsViewManager::Refresh(nsView*, nsIntRegion const&) nsViewManager::PaintWindow(nsIWidget*, nsIntRegion, unsigned int) nsView::PaintWindow(nsIWidget*, nsIntRegion, unsigned int) nsChildView::PaintWindow(nsIntRegion, bool) -[ChildView drawRect:inContext:alternate:] -[ChildView drawRect:inTitlebarContext:] -[ChildView maybeDrawInTitlebar] -[ToolbarWindow setTitlebarNeedsDisplayInRect:sync:] -[ToolbarWindow setTitlebarNeedsDisplayInRect:] -[ChildView setNeedsDisplayInRect:] -[NSView setNeedsDisplay:] [...] -[NSWindow setFrame:] Now it's fired from this callstack: DelayedFireDOMPaintEvent::Run() nsContentUtils::RemoveScriptBlocker() nsViewManager::Refresh(nsView*, nsIntRegion const&) nsViewManager::PaintWindow(nsIWidget*, nsIntRegion) nsView::PaintWindow(nsIWidget*, nsIntRegion) nsChildView::PaintWindow(nsIntRegion) -[ChildView drawUsingOpenGL] -[ChildView drawRect:inContext:] -[ChildView drawRect:] [...] -[NSView displayIfNeeded] [...] -[NSWindow setFrame:]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
OK, thanks. I thought perhaps UX was faster than m-c before for some other reason but we didn't have talos running on UX the whole time so I couldn't see what caused the decrease. Thanks for following up.
Note that in the screenshot (and actual graph server data), it still looks like there's a slight regression... Eg, in attachment 759549 [details] the red dots are above the level of the green dots, but it's noisy. However, the graph server data only shows that for 10.6. The perf numbers for 10.7 and 10.8 are clearly at the same level (no regression). So I don't think the 10.6 data is significant.
Yes, I had missed the general regression on 10.6. Matt filed bug 880620 on it.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: