Bug 216498

Summary: [Cocoa] Prototype fontd-less WebKit
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: TextAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED MOVED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, ews-watchlist, mmaxfield, nham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Microbenchmark
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Microbenchmark
none
WIP
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Myles C. Maxfield
Reported 2020-09-14 15:16:12 PDT
Let's see if there are any perf wins.
Attachments
Microbenchmark (882 bytes, text/html)
2020-09-14 15:17 PDT, Myles C. Maxfield 		no flags
Details
Patch (42.32 KB, patch)
2020-09-15 01:28 PDT, Myles C. Maxfield 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.32 KB, patch)
2020-09-15 01:32 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (42.38 KB, patch)
2020-09-15 02:03 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Microbenchmark (1.06 KB, text/html)
2020-09-15 02:16 PDT, Myles C. Maxfield 		no flags
Details
WIP (40.82 KB, patch)
2020-09-16 00:17 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (42.55 KB, patch)
2020-09-16 22:49 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.19 KB, patch)
2020-09-17 00:33 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.28 KB, patch)
2020-09-17 00:43 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.32 KB, patch)
2020-09-17 00:52 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.30 KB, patch)
2020-09-17 00:57 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.28 KB, patch)
2020-09-17 01:00 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.46 KB, patch)
2020-09-17 01:20 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (36.31 KB, patch)
2020-09-17 15:13 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (36.24 KB, patch)
2020-09-17 17:56 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (36.65 KB, patch)
2020-09-18 15:00 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (36.63 KB, patch)
2020-09-18 22:50 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.45 KB, patch)
2021-01-14 17:56 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (33.47 KB, patch)
2021-01-15 00:37 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Show Obsolete (17) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2020-09-14 15:17:16 PDT
Created attachment 408751 [details] Microbenchmark On this microbenchmark, we spend 67% of the time looking up fonts (inside CTFontDescriptorCreateMatchingFontDescriptor() or CTFontDescriptorCreateMatchingFontDescriptors()).
Myles C. Maxfield
Comment 2 2020-09-15 01:28:56 PDT
Created attachment 408801 [details] Patch
Myles C. Maxfield
Comment 3 2020-09-15 01:32:44 PDT
Created attachment 408802 [details] Patch
Myles C. Maxfield
Comment 4 2020-09-15 02:03:20 PDT
Created attachment 408805 [details] Patch
Myles C. Maxfield
Comment 5 2020-09-15 02:16:45 PDT
Created attachment 408806 [details] Microbenchmark
Myles C. Maxfield
Comment 6 2020-09-15 02:38:59 PDT
This is a 2.2x speedup on the microbenchmark!! Here are my numbers, units are ms, lower is better: With patch: 1205 1201 1250 1272 1240 1250 1260 1249 1230 1233 Without patch: 3993 3928 3952 3987 3954 3967 3958 4048 3915 4030
Myles C. Maxfield
Comment 7 2020-09-15 21:11:34 PDT
Remaining tasks: 1. Make this patch do nothing on non-macOS ports 2. Iterate over all fonts, not just the ones returned by an empty dictionary and CTFontDescriptorCreateMatchingFontDescriptors() 3. Make sure tests pass 4. Most tests allow user-installed fonts, which won't hit this codepath. Do something to be more confident in this patch
Myles C. Maxfield
Comment 8 2020-09-16 00:17:32 PDT
Created attachment 408902 [details] WIP
Myles C. Maxfield
Comment 9 2020-09-16 22:49:23 PDT
Created attachment 408991 [details] Patch
Myles C. Maxfield
Comment 10 2020-09-17 00:33:02 PDT
Created attachment 409001 [details] Patch
Myles C. Maxfield
Comment 11 2020-09-17 00:39:29 PDT
Remaining tasks: 1. Make sure tests pass 2. Most tests allow user-installed fonts, which won't hit this codepath. Do something to be more confident in this patch 3. Key the fonts on all localizations of the family name
Myles C. Maxfield
Comment 12 2020-09-17 00:43:17 PDT
Created attachment 409002 [details] Patch
Myles C. Maxfield
Comment 13 2020-09-17 00:52:50 PDT
Created attachment 409004 [details] Patch
Myles C. Maxfield
Comment 14 2020-09-17 00:57:55 PDT
Created attachment 409005 [details] Patch
Myles C. Maxfield
Comment 15 2020-09-17 01:00:47 PDT
Created attachment 409006 [details] Patch
Myles C. Maxfield
Comment 16 2020-09-17 01:20:05 PDT
Created attachment 409009 [details] Patch
Myles C. Maxfield
Comment 17 2020-09-17 14:05:51 PDT
See also: <rdar://problem/68579484>
Myles C. Maxfield
Comment 18 2020-09-17 15:13:01 PDT
Created attachment 409069 [details] Patch
Myles C. Maxfield
Comment 19 2020-09-17 15:56:36 PDT
Remaining task: Most tests allow user-installed fonts, which won't hit this codepath. Do something to be more confident in this patch
Myles C. Maxfield
Comment 20 2020-09-17 17:56:51 PDT
Created attachment 409097 [details] Patch
Myles C. Maxfield
Comment 21 2020-09-18 15:00:45 PDT
Created attachment 409172 [details] Patch
Myles C. Maxfield
Comment 22 2020-09-18 22:50:28 PDT
Created attachment 409196 [details] Patch
Myles C. Maxfield
Comment 23 2020-09-20 23:35:20 PDT
Integrating this into Apple's build system will be quite challenging. I'll be putting this on hold to see if it's possible to solve this another way. See also: <rdar://problem/68579484>
Radar WebKit Bug Importer
Comment 24 2020-09-21 15:17:16 PDT
<rdar://problem/69330831>
Myles C. Maxfield
Comment 25 2020-09-22 13:39:46 PDT
I also filed <rdar://problem/69389335>
Ben Nham
Comment 26 2020-11-11 15:31:30 PST
Comment on attachment 409196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409196&action=review > Source/WebCore/platform/graphics/cocoa/PreinstalledFonts.mm:119 > + prebuiltFontFamilyMembers(familyName); I think this should be `return prebuiltFontFamilyMembers(familyName)`. After making that change I got the expected speedup from prototyping the patch.
Ben Nham
Comment 27 2020-11-11 15:31:52 PST
Comment on attachment 409196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409196&action=review > Source/WebCore/platform/graphics/cocoa/PreinstalledFonts.mm:119 > + prebuiltFontFamilyMembers(familyName); I think this should be `return prebuiltFontFamilyMembers(familyName)`. After making that change I got the expected speedup from prototyping the patch.
Myles C. Maxfield
Comment 28 2021-01-14 17:56:58 PST
Created attachment 417673 [details] Patch
Myles C. Maxfield
Comment 29 2021-01-15 00:37:43 PST
Created attachment 417684 [details] Patch
Myles C. Maxfield
Comment 30 2021-03-12 18:27:11 PST
This is fixed by rdar://68579484.
Note You need to log in before you can comment on or make changes to this bug.