Closed Bug 36485 Opened 25 years ago Closed 25 years ago

Batch reflows when loading user prefs

Categories

(Core Graveyard :: Profile: BackEnd, defect, P2)

Product:
Core Graveyard
Component:
Profile: BackEnd
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: pierre, Assigned: eric)

References

Details

(Keywords: perf)

This bug is extracted from the email thread we had yesterday on <c- t@netscape.com> ("Please Review: Reflow Counts Statistics"). sfraser wrote: ---- As the profile manager sets up user preferences, the pref callbacks in nsPresContext get called for each of the following preferences (for me) (I added a printf in nsPresContext::PreferenceChanged() to catch these): Reflow because pref browser.background_color changed Reflow because pref browser.use_document_colors changed Reflow because pref browser.wfe.use_windows_colors changed Reflow because pref font.size.variable.ar changed Reflow because pref font.size.fixed.ar changed Reflow because pref font.size.variable.el changed Reflow because pref font.size.fixed.el changed Reflow because pref font.size.variable.he changed Reflow because pref font.size.fixed.he changed Reflow because pref font.size.variable.ja changed Reflow because pref font.size.fixed.ja changed Reflow because pref font.size.variable.ko changed Reflow because pref font.size.fixed.ko changed Reflow because pref font.size.variable.th changed Reflow because pref font.size.fixed.th changed Reflow because pref font.size.variable.tr changed Reflow because pref font.size.fixed.tr changed Reflow because pref font.size.variable.x-baltic changed Reflow because pref font.size.fixed.x-baltic changed Reflow because pref font.size.variable.x-central-euro changed Reflow because pref font.size.fixed.x-central-euro changed Reflow because pref font.size.variable.x-cyrillic changed Reflow because pref font.size.fixed.x-cyrillic changed Reflow because pref font.size.variable.x-unicode changed Reflow because pref font.size.fixed.x-unicode changed Reflow because pref font.size.variable.x-western changed Reflow because pref font.size.fixed.x-western changed Reflow because pref font.size.variable.zh-CN changed Reflow because pref font.size.fixed.zh-CN changed Reflow because pref font.size.variable.zh-TW changed Reflow because pref font.size.fixed.zh-TW changed Reflow because pref browser.use_document_fonts changed Reflow because pref font.name.serif.ar changed Reflow because pref font.name.sans-serif.ar changed Reflow because pref font.name.monospace.ar changed Reflow because pref font.name.cursive.ar changed Reflow because pref font.name.fantasy.ar changed Reflow because pref font.name.serif.el changed Reflow because pref font.name.sans-serif.el changed Reflow because pref font.name.monospace.el changed Reflow because pref font.name.cursive.el changed Reflow because pref font.name.fantasy.el changed Reflow because pref font.name.serif.he changed Reflow because pref font.name.sans-serif.he changed Reflow because pref font.name.monospace.he changed Reflow because pref font.name.cursive.he changed Reflow because pref font.name.fantasy.he changed Reflow because pref font.name.serif.ja changed Reflow because pref font.name.sans-serif.ja changed Reflow because pref font.name.monospace.ja changed Reflow because pref font.name.cursive.ja changed Reflow because pref font.name.fantasy.ja changed Reflow because pref font.name.serif.ko changed Reflow because pref font.name.sans-serif.ko changed Reflow because pref font.name.monospace.ko changed Reflow because pref font.name.cursive.ko changed Reflow because pref font.name.fantasy.ko changed Reflow because pref font.name.serif.th changed Reflow because pref font.name.sans-serif.th changed Reflow because pref font.name.monospace.th changed Reflow because pref font.name.cursive.th changed Reflow because pref font.name.fantasy.th changed Reflow because pref font.name.serif.tr changed Reflow because pref font.name.sans-serif.tr changed Reflow because pref font.name.monospace.tr changed Reflow because pref font.name.cursive.tr changed Reflow because pref font.name.fantasy.tr changed Reflow because pref font.name.serif.x-baltic changed Reflow because pref font.name.sans-serif.x-baltic changed Reflow because pref font.name.monospace.x-baltic changed Reflow because pref font.name.cursive.x-baltic changed Reflow because pref font.name.fantasy.x-baltic changed Reflow because pref font.name.serif.x-central-euro changed Reflow because pref font.name.sans-serif.x-central-euro changed Reflow because pref font.name.monospace.x-central-euro changed Reflow because pref font.name.cursive.x-central-euro changed Reflow because pref font.name.fantasy.x-central-euro changed Reflow because pref font.name.serif.x-cyrillic changed Reflow because pref font.name.sans-serif.x-cyrillic changed Reflow because pref font.name.monospace.x-cyrillic changed Reflow because pref font.name.cursive.x-cyrillic changed Reflow because pref font.name.fantasy.x-cyrillic changed Reflow because pref font.name.serif.x-unicode changed Reflow because pref font.name.sans-serif.x-unicode changed Reflow because pref font.name.monospace.x-unicode changed Reflow because pref font.name.cursive.x-unicode changed Reflow because pref font.name.fantasy.x-unicode changed Reflow because pref font.name.serif.x-western changed Reflow because pref font.name.sans-serif.x-western changed Reflow because pref font.name.monospace.x-western changed Reflow because pref font.name.cursive.x-western changed Reflow because pref font.name.fantasy.x-western changed Reflow because pref font.name.serif.zh-CN changed Reflow because pref font.name.sans-serif.zh-CN changed Reflow because pref font.name.monospace.zh-CN changed Reflow because pref font.name.cursive.zh-CN changed Reflow because pref font.name.fantasy.zh-CN changed Reflow because pref font.name.serif.zh-TW changed Reflow because pref font.name.sans-serif.zh-TW changed Reflow because pref font.name.monospace.zh-TW changed Reflow because pref font.name.cursive.zh-TW changed Reflow because pref font.name.fantasy.zh-TW changed Reflow because pref browser.screen_resolution changed Reflow because pref font.default changed Reflow because pref font.size.fixed.x-western changed Reflow because pref font.size.variable.x-western changed This is a well-known problem, I believe. ---- And buster replied: ---- Isn't this exactly the kind of thing nsIPresShell::BeginReflowBatching() is designed for? When you know you're going to do a bunch of work, and it's unimportant to display partial results, we should be able to collect the reflow requests and handle them all at once when the caller is done making changes. ---
Keywords: perf
Hmmm. I would have thought this stuff is happening when there isn't even a window open. Why are there reflows going on at all? We're certainly not showing the prefs panel during startup! What is the recommended fix for this bug? Is it actually the profile manager that needs to change?
Marked P2/M16. This problem has nothing to do with the Preferences dialog. I assigned this bug to "Profile Manager BackEnd" because it seemed to be the most logical component. Maybe it should go to "Preferences Backend", I don't know. The thing is: we start the application with default prefs, the Profile manager is displayed, the user selects a profile, then we load the rest of the application with the preferences that correspond to the selected profile. When we load either prefs (the default ones or the selected ones), we should tell the PresShell to batch the reflows. That's the bug. My guess was that the module in charge of loading the prefs was the Profile Manager. If I'm wrong, please reassign.
Priority: P3 → P2
Target Milestone: --- → M16
Neeti, this is really a prefs thing.
Assignee: selmer → neeti
The preference callback nsPresContext::PreferenceChanged() is being called "only" for the prefs which are registered for callbacks by the nsPresContext and not all the prefs in all.js( there are lots more prefs in all.js for which which the callback is not called). The prefs which have been registered by nsPresContext are mPrefs->RegisterCallback("browser.screen_resolution", PrefChangedCallback, (void*)this); D:\seamonkey\mozilla\layout\base\src\nsPresContext.cpp(308): mPrefs->RegisterCallback("browser.use_document_fonts", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.use_document_colors", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.background_color", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.foreground_color", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.base_font_scaler", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.wfe.use_windows_colors", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("font.", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("browser.display.direction", PrefChangedCallback, (void*)this); pref->RegisterCallback("intl.charset.detector", MyPrefChangedCallback, nsnull); I agree, we are getting a lot of callbacks(52) for nsPresContext::PreferenceChanged(), but libpref sends those callbacks, only if you have registered for them. Libpref has no ability to tell PresShell to batch the reflows. I believe, evaughan has a fix for this problem. Neeti
Assignee: neeti → pierre
Yes I do have a fix for this. I'll close out when I check in. Reassigning to me.
Assignee: pierre → evaughan
*** Bug 36015 has been marked as a duplicate of this bug. ***
Eric: Nisheeth proposed a solution where the pref callbacks are ignored until BeginLoad() is called. I don't how that fits with your strategy.
Not sure. My solution is more of a global solution that works for all StyleChanges everywhere not just the initial prefs. It currently works quite well.
Status: NEW → ASSIGNED
fixed
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
sairuh, Should this bug be yours?
QA Contact: gbush → sairuh
no longer see this problem. verif.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.