I don't know the what everything is called so this might be a misunderstanding, but anyway, I'm looking at performance at the very problematic DTML site in bug 124178, and nsViewManager::UpdateView is called a lot (from nsViewManager::ResizeView) for a total of 22% of the time. Those 22% of the time is spent in nsViewManager::UpdateCoveringWidgets and as far as I can tell there are no widgets at all the page so it looks a little strange. Also, that function does a lot of recursive calls to itself. Maybe a iterative solution would be more efficient?

UpdateCoveringWidgets definitely needs a makeover. It doesn't take account of widget z-ordering, which means it often invalidates widgets that don't need to be invalidated. And as you observe, it's too slow; when few views have widgets, we traverse nearly the entire view tree.

Here's the fix. It's a little bit complicated because I found that widgets were not properly being organized into a tree; the widget for the root of an IFRAME document was not being made a child of a widget in the outer document. So I fixed that, although I had to add a flag to tell widget/gtk to post resize events for those child widgets. (I eyeballed Mac and Win and they don't seem to need to use this flag.) The guts of the patch converts UpdateAllCoveringWidgets from traversing the view tree to traversing the widget tree. There are usually far fewer widgets than views so this means we do less work. It also simplifies the code a bit. Note that the code which checked for non-blittable internal views and forces them to update has been removed. The patch in bug 124554 takes over the check for non-blittable internal views.

Sorry, some debug code slipped in. This patch is good.

OK, so I was testing the previous fix and I found a bug in the way it handled scrolling. We ended up repainting everything. This fixes that, so scrolling will most just scroll and not paint.

Daniel, I just ran jprof on that URL and I only saw about 1% of the time being spent in UpdateCoveringWidgets with our existing code. Not sure why it would be different to your run. Would you mind trying this patch and seeing if it makes a significant different in your setup? ("UpdateCoveringWidgets" has been renamed to "UpdateWidgetArea".) Thanks!

Pushing to 1.1. If someone can prove this is really important for performance and this patch helps, then I can pull it back to 1.0.

Windows only thing? Something caused by running Mozilla in Quantify? I have not been able to make profiling builds since the switch to the new build system so I can not confirm this right now. If I can confirm that the patch improves performance I will try to get it in 1.0, but a target of 1.1 is good until then.

Robert's patch effective fixing this bug: http://bugzilla.mozilla.org/show_bug.cgi?id=148212 Request patch against current/recent code be made/posted. I think I did okay merging it, but sure author do better.

Let's get reviews for this and see if we can get it into 1.1alpha.

Ccing review buddies

Comment on attachment 86114 [details] [diff] [review] Up-to-date patch small nit: Shouldn't listenForResizes be named mListenForResizes? r=kmcclusk@netscape.com

Comment on attachment 86114 [details] [diff] [review] Up-to-date patch rs=waterson

I'll rename it to mListenForResizes. Thanks!

Fix checked in.

Think this one caused some frame-drop offs - seen in http://www.world-direct.com/mozilla/dhtml/75121/anim-test.htm using trunk build 2002061204

or a better example: http://www.formula-one.nu/dhtml/fish/fish2.htm

Could this one have caused the regression bug 153083 ?

Exactly after this checkin frame-drop offs started to happen! see bug 157401. we need jprofs with/without the 124685 patch.

Would be nice if somebody could answer the question that Markus did 46 days ago in comment 15. A simple yes and no would do.

Comment 15 does not look like a question. Comment 17 does, but the discussion of what have caused the regression is (naturally) happening in the comments of that bug, not here.