Open Bug 1377352 Opened 2 years ago Updated 2 years ago

nsXPLookAndFeel::GetIntImpl etc are slow


(Core :: Widget, defect, P3, minor)





(Reporter: mats, Unassigned)



(Keywords: perf, Whiteboard: tpi:+)

I'm looking at the perf profile for Gmail in bug 1371668 comment 0 and
the following stack (inverted) shows up there:

nsLookAndFeel::GetIntImpl(mozilla::LookAndFeel::IntID,int &)
nsXPLookAndFeel::GetIntImpl(mozilla::LookAndFeel::IntID,int &)

I suspect the problem here is that the compiler can't devirtualize
these calls.
Unfortunately though, bug 1338004 changed:
to return the base class rather than the nsLookAndFeel leaf class,
so making nsLookAndFeel 'final' (bug 1377348) probably won't help
this bug. :(
It might work if we move nsXPLookAndFeel::GetInstance() to the header
so that it can be inlined:
and then split 'sInstance' into two separate variables, each having
the concrete types, then return those in the branches, something like:

  static nsLookAndFeel sInstanceLookAndFeel;
  static widget::HeadlessLookAndFeel sInstanceHeadless;
  if (sInstanceLookAndFeel) {
    return sInstanceLookAndFeel;
  } else if (!gfxPlatform::IsHeadless()) {
    return sInstanceLookAndFeel = new nsLookAndFeel();
  } else if (sInstanceHeadless) {
    return sInstanceHeadless;
  } else {
    return sInstanceHeadless = new widget::HeadlessLookAndFeel();

then the compiler might be able to call the concrete method
based on which branch was taken...
It seems odd that this is showing up in profiles as I see it's only called ~10k times while browsing gmail and youtube for a few minutes. There is a loop in nsXPLookAndFeel::GetIntImpl over the preference stored values but that's also pretty small.

I'm not sure it's possible, but we could reduce a number of layers here if we instead used a single lookup in an array (there are ~53 ints) of all the cached int values and then relied on each platform specific lookAndFeel to update these values.
Priority: -- → P3
Whiteboard: tpi:+
You need to log in before you can comment on or make changes to this bug.