Closed Bug 36485 Opened 24 years ago Closed 24 years ago

Batch reflows when loading user prefs

Categories

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

defect

Tracking

(Not tracked)

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: 24 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.