Closed
Bug 44162
Opened 24 years ago
Closed 21 years ago
Should batch reflows and turn off painting around UpdateCommands
Categories
(Core :: XUL, defect, P3)
Core
XUL
Tracking
()
RESOLVED
WONTFIX
Future
People
(Reporter: sfraser_bugs, Assigned: sfraser_bugs)
Details
(Keywords: helpwanted, perf)
Since code called by UpdateCommands can cause lots of attribute setting, which can cause reflows and painting, we should batch reflows and view manager updates around this call.
Assignee | ||
Comment 1•24 years ago
|
||
Here's a diff to fix this. This makes quite an impact on editor window loading. Index: nsGlobalWindow.cpp =================================================================== RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v retrieving revision 1.307 diff -u -2 -r1.307 nsGlobalWindow.cpp --- nsGlobalWindow.cpp 2000/06/27 22:13:09 1.307 +++ nsGlobalWindow.cpp 2000/06/29 00:35:00 @@ -1829,5 +1829,25 @@ nsCOMPtr<nsIDOMXULCommandDispatcher> xulCommandDispatcher; xulDoc->GetCommandDispatcher(getter_AddRefs(xulCommandDispatcher)); + + // Batch reflows + PRBool wasBatching = PR_FALSE; + nsCOMPtr<nsIPresShell> presShell; + nsCOMPtr<nsIViewManager> viewManager; + + mDocShell->GetPresShell(getter_AddRefs(presShell)); + if (presShell) { + presShell->GetViewManager(getter_AddRefs(viewManager)); + if (viewManager) + viewManager->BeginUpdateViewBatch(); + presShell->GetReflowBatchingStatus(&wasBatching); + presShell->BeginReflowBatching(); + } + xulCommandDispatcher->UpdateCommands(anAction); + + if (presShell) + presShell->EndReflowBatching(wasBatching); + if (viewManager) + viewManager->EndUpdateViewBatch(0); }
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•24 years ago
|
||
waterson: can I get this in by you as a mozilla.org checkin? It makes a big difference for the composer window.
Target Milestone: --- → M18
Comment 4•24 years ago
|
||
Well, since you're not a "non-Netscape contributor", no, I'm not going pretend you are. But I'm not your boss, nor am I the police. If you think it makes a big enough difference and are willing to risk a "talking to" from the PDT, then check it in. The patch certainly seems extremely reasonable.
Comment 5•24 years ago
|
||
Wouldn't it be better to do this inside the command dispatcher's UpdateCommands method? Then everyone benefits, not just people who use the DOM window method.
Assignee | ||
Comment 6•24 years ago
|
||
If I did this in nsXULCommandDispatcher::UpdateCommands, I'd have to do it inside the Updater loop (where I have a presShell). That means that I might be turning batching on and off multiple times per presShell. I guess I could run through the Updaters first, and turn on batching once per unique presShell, then off again at the end. But that means more code.
Comment 8•24 years ago
|
||
please do not nominate for nsbeta3 -- this will be addressed after crashes, regressions, correctness and polish bugs have been resolved, all Composer perf bugs have been set to m19/m20
Comment 9•24 years ago
|
||
moving to future per review with bij and beppe
Keywords: helpwanted
Target Milestone: M19 → Future
Comment 11•22 years ago
|
||
removing myself from the cc list
Comment 12•22 years ago
|
||
What's the approach on that now? Seems to be really reasonable.
Assignee | ||
Comment 13•22 years ago
|
||
This bug is pretty stale now. I'm not sure if it's still a problem, or if the patch is worthwhile.
Assignee: sfraser → jaggernaut
Status: ASSIGNED → NEW
Comment 14•21 years ago
|
||
-> sfraser Simon, what do you want to do with this?
Assignee: jaggernaut → sfraser
Assignee | ||
Comment 15•21 years ago
|
||
Oldy crufy.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•