While reviewing attachment 802685 [details] [diff] [review] (which adds a function much like FlexboxEnabledPrefChangeCallback), I was inspired to move FlexboxEnabledPrefChangeCallback's static variables into its function scope. Patch coming up to do that, and do the same for a few other static variables. (and also adds an assertion analogous to the assertion I requested on attachment 802685 [details] [diff] [review])
function-scope static variables are more expensive than global-scope ones (at least if they require any non-null initialization), since they're initialized the first time the function is called, which requires an extra hidden (compiler-generated) boolean to guard.
Ah, good to know - thanks! Nearly all of the variables I was going to move use null initialization (or equivalent, e.g. "= false", which could be removed if it actually imposes a cost). I'll hold off on moving sScrollIdCounter, though, since that one's initialized to 2.
(In reply to David Baron [:dbaron] (needinfo? me) from comment #1) > function-scope static variables are more expensive than global-scope ones That's a good point, but these pref callbacks are only called once on startup, and when the pref changes, so I don't think we need to worry about performance in this case. If we put the sScrollIdCounter definition inside the "if (!FindIDFor(aContent, &scrollId))" we will only do the guard check once per content object the method is called with, which I think is negligible. http://mxr.mozilla.org/mozilla-central/search?string=FindOrCreateIDFor
Comment on attachment 806955 [details] [diff] [review] fix v1 If you agree with my last comment, then please move the sScrollIdCounter inside the 'if'. r=mats
Attachment #806955 - Flags: review?(matspal) → review+
I'll leave sScrollIdCounter alone for now, since it's doing something different and it's not as obviously-worth-fixing as the other variables. So, I'll just land the patch as-is. If anyone cares about sScrollIdCounter being function-scoped, we can make that change in a separate bug (or separate patch on this bug). :)
Summary: For static variables in nsLayoutUtils.cpp that only get used in one function, move them to that function's scope → For null-initialized static variables in nsLayoutUtils.cpp that only get used in one function, move them to that function's scope
Tree's closed; using checkin-needed.
Tree reopened! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/8367f4c4fb71
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.