Closed Bug 688193 Opened 13 years ago Closed 1 year ago

startup perf- Remove DisableFontActivation from critical startup path on Mac

Categories

(Core :: Graphics: Text, defect)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
112 Branch
Tracking Status
firefox112 --- fixed

People

(Reporter: Yoric, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(3 files)

We are currently in the process of hunting down the last few remaining fsync that appear at startup. According to bug 611837, on the Macintosh, there are only two such occurrences of fsync, and one of them is DisableFontActivation.

Discussing with jfkthame, it seems that this operation could be moved away from the critical path.

Pretty please?
AFAICS from bug 567552, the use of DisableFontActivation relates only to the use of @font-face with src:local(...), as that's the only situation where we query the OS for a font name (as opposed to looking up fonts using our own font list).

Thus, we don't actually need to tweak the setting during platform startup; we can defer it until the first time gfxMacPlatformFontList::LookupLocalFont() gets called.
Assignee: nobody → jfkthame
Attachment #561509 - Flags: review?(jdaggett)
Comment on attachment 561509 [details] [diff] [review]
patch, remove DisableFontActivation from startup path and do it on-demand instead

> AFAICS from bug 567552, the use of DisableFontActivation relates only to the
> use of @font-face with src:local(...), as that's the only situation where we
> query the OS for a font name (as opposed to looking up fonts using our own
> font list).

Not sure what made you think that but that's *not* the case.  The bug
occurs on startup on 10.6 machines with external hard drives that
contain folders with fonts when the font list is built.  My guess is
that the underlying bug is some form of Spotlight bug.  So delaying
this will bring back the original *very* problematic behavior of bug
567552.

Another approach would be to switch to using the CTFontManager
enumeration calls instead of NSFontManager.  My guess is that the
CoreText API's don't have this problem, otherwise the same problem
would occur with Safari.  This API is 10.6 and beyond, so we'd need to
keep the old code for 10.5 support.
Attachment #561509 - Flags: review?(jdaggett) → review+
Attachment #561509 - Flags: review+ → review-
Are you sure it's known to occur when the font list is built? I'd find that very surprising, given that we get the font list from Cocoa's NSFontManager, which is the preferred, Apple-sanctioned way to manage fonts. If this can be triggered by calling
    sFontManager = [NSFontManager sharedFontManager];
to get the font manager, and then
    NSEnumerator *families =
        [[sFontManager availableFontFamilies] objectEnumerator];
to get an enumerator and iterate over the list, which is what we do, then MANY Cocoa applications should be exhibiting the same issue, as this is The Cocoa Way to find the available fonts. Surely this cannot cause the OS to suddenly look for specific named, non-installed fonts.

I suspect that when people have seen this "at startup", it's because they have tabs in their saved session that happen to include @font-face { src:local(...) } rules. AFAICS, this is the only scenario where we call font-querying APIs (either ATS or CG-based, depending on OS version) that could trigger the OS to wish to "activate" a currently-uninstalled font by name.
This is what I get with about:blank as a startup page. This is on a nightly a few days old.
(In reply to David Rajchenbach Teller [:Yoric] from comment #4)
> Created attachment 561993 [details]
> Stack leading to DisableFontActivation
> 
> This is what I get with about:blank as a startup page. This is on a nightly
> a few days old.

Yes - we currently call DisableFontActivation to change the OS setting during startup, because of bug 567552. What I meant in comment 3 is that I don't see how bug 567552 could really be happening at startup, but must be triggered by @font-face in a page that's being loaded (which could be part of session restore, so the user would see it as "at startup") - and therefore, we could defer DisableFontActivation until we encounter such a rule, moving it out of the actual startup path.
Severity: normal → S3
Component: Graphics → Graphics: Text

It shouldn't be necessary to do this at startup, only on first use of LookupLocalFont.
(We can also simplify things a bit: now that we don't support pre-10.6 systems where
the symbol wasn't present, we don't need to do a runtime dlsym() check.)

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a8ced3e4470
Remove call to CTFontManagerSetAutoActivationSetting from the startup path on macOS. r=gfx-reviewers,lsalzman
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: