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)
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?
Assignee | ||
Comment 1•13 years ago
|
||
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 2•13 years ago
|
||
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+
Updated•13 years ago
|
Attachment #561509 -
Flags: review+ → review-
Assignee | ||
Comment 3•13 years ago
|
||
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.
Reporter | ||
Comment 4•13 years ago
|
||
This is what I get with about:blank as a startup page. This is on a nightly a few days old.
Assignee | ||
Comment 5•13 years ago
|
||
(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.
Updated•2 years ago
|
Severity: normal → S3
Updated•1 year ago
|
Component: Graphics → Graphics: Text
Assignee | ||
Comment 6•1 year ago
|
||
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
Comment 8•1 year ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 1 year ago
status-firefox112:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•