Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1683883002:
Add request cache to SkFontHost_fontconfig. (Closed)
|
Created:
4 years, 10 months ago by bungeman-skia Modified:
4 years, 10 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionAdd request cache to SkFontHost_fontconfig.
The current code deduplicates SkTypeface instances as all font lookups
should for better use of the glyph cache. This adds a request cache as
well, so that repeated recent requests will return the cached result
instead of doing a full lookup.
BUG=chromium:424082, chromium:444894
Committed: https://skia.googlesource.com/skia/+/70bb80802198b7fedee58f79d0d68d5f8aba8b62
Patch Set 1 #
Total comments: 16
Patch Set 2 : Address comments. #Patch Set 3 : Use SkResourceCache instead. #
Total comments: 2
Patch Set 4 : Fix deallocation. #Patch Set 5 : Clean up, assert the right things. #Patch Set 6 : 'this' is already sufficiently aligned, just check length. #Patch Set 7 : No longer need 'memory' include. #
Total comments: 2
Patch Set 8 : Change to 'size', clarify comments. #
Total comments: 2
Patch Set 9 : Make method 'const'. #
Messages
Total messages: 56 (28 generated)
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/1
bungeman@google.com changed reviewers: + kochi@chromium.org, mtklein@google.com, reed@google.com
This is an alternative to https://crrev.com/838743002 .
The CQ bit was unchecked by bungeman@google.com
lgtm The performance with this fix is quite nice. Thanks for working on this.
https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... File src/ports/SkFontHost_fontconfig.cpp (left): https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:95: // Check if requested NameStyle is in the NameStyle cache. Needs an update / deletion? https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:98: SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_NameStyle, &nameStyle); Is this new request-caching approach something we should spread over to SkFontHost_mac too? I see it's the only user of find_by_NameStyle now. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... File src/ports/SkFontHost_fontconfig.cpp (right): https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:72: fCachedResults.emplace_back(face, Request(requestedFamilyName, requestedStyle)); I found myself wondering whether or not these typefaces were refcounted when reading through here, up until I read the Result struct. It might be clearer that we're reffing the typefaces if we move the ref from the constructor to each of these emplace_back calls explicitly: fCachedResults.emplace_back(SkSafeRef(face), Request(...)); https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:75: fLRUIndex++; fLRUIndex seems like the wrong name here. There's no LRU going on... we're not tracking when things are used, only when they're added. fRingNext, or just fNext? https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:103: static SkFontHostRequestCache& Get() { add gSKFontHostRequestCacheMutex.assertHeld(); ? Gotta worry about racing on the static constructor, right? https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:108: // Set the maxEntries to a small value in debug to ensure testing of wrap around. // 64 for Release builds is... (arbitrary?) (pulled from a histogram of web content?) (a nice round number?) https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:119: Result(SkTypeface* typeface, Request&& request) I'd slightly prefer passing (Request&&, SkTypeface*) to make it read more like a key->value mapping. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:163: // Add this NameStyle to the NameStyle cache. Update? Seems weird when there's no NameStyle object here anymore.
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
After having written all that... lets try something completely different. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... File src/ports/SkFontHost_fontconfig.cpp (left): https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:95: // Check if requested NameStyle is in the NameStyle cache. On 2016/02/10 16:14:10, mtklein wrote: > Needs an update / deletion? Done. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:98: SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_NameStyle, &nameStyle); On 2016/02/10 16:14:10, mtklein wrote: > Is this new request-caching approach something we should spread over to > SkFontHost_mac too? I see it's the only user of find_by_NameStyle now. Yeah, SkFontHost_mac has its own find_by_NameStyle which is somewhat suspicious. I would prefer not to do any of this request caching, or if we do to provide it as a separable wrapper (a SkFontMgr_request_caching). This is a somewhat special case just for Chromium/Blink. It's been a long term goal to fully separate the request cache from the typeface cache, and this is a nice step in that direction. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... File src/ports/SkFontHost_fontconfig.cpp (right): https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:72: fCachedResults.emplace_back(face, Request(requestedFamilyName, requestedStyle)); On 2016/02/10 16:14:10, mtklein wrote: > I found myself wondering whether or not these typefaces were refcounted when > reading through here, up until I read the Result struct. > > It might be clearer that we're reffing the typefaces if we move the ref from the > constructor to each of these emplace_back calls explicitly: > fCachedResults.emplace_back(SkSafeRef(face), Request(...)); Hmmm... so the reason things are written the way they are here is that normally passing a bare pointer to a SkRefCnt means no transfer of the reference count. (On the convention that if the callee needs to keep a reference it will increment the reference count itself.) I also find it easier to reason about that Result does all the refing and unrefing itself. If the Request and Result types were defined first, does that make things a bit clearer? There are a few odd places where we document passing the bare pointer does transfer the reference count, but I would prefer that those eventually get updated to some sort of move based system so that the types indicate this is happening rather than a comment. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:75: fLRUIndex++; On 2016/02/10 16:14:10, mtklein wrote: > fLRUIndex seems like the wrong name here. There's no LRU going on... we're not > tracking when things are used, only when they're added. fRingNext, or just > fNext? Yeah, it's actually the fLRAIndex (least recently added). The issue with just fNextIndex is that it isn't the next index in the 'warm up'. It really is the oldest added entry. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:103: static SkFontHostRequestCache& Get() { On 2016/02/10 16:14:10, mtklein wrote: > add > gSKFontHostRequestCacheMutex.assertHeld(); > > ? Gotta worry about racing on the static constructor, right? Done. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:108: // Set the maxEntries to a small value in debug to ensure testing of wrap around. On 2016/02/10 16:14:10, mtklein wrote: > // 64 for Release builds is... (arbitrary?) (pulled from a histogram of web > content?) (a nice round number?) A totally magic number. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:119: Result(SkTypeface* typeface, Request&& request) On 2016/02/10 16:14:10, mtklein wrote: > I'd slightly prefer passing (Request&&, SkTypeface*) to make it read more like a > key->value mapping. Done. https://codereview.chromium.org/1683883002/diff/1/src/ports/SkFontHost_fontco... src/ports/SkFontHost_fontconfig.cpp:163: // Add this NameStyle to the NameStyle cache. On 2016/02/10 16:14:10, mtklein wrote: > Update? Seems weird when there's no NameStyle object here anymore. Done.
https://codereview.chromium.org/1683883002/diff/40001/src/ports/SkFontHost_fo... File src/ports/SkFontHost_fontconfig.cpp (right): https://codereview.chromium.org/1683883002/diff/40001/src/ports/SkFontHost_fo... src/ports/SkFontHost_fontconfig.cpp:96: char* storage = new char [sizeof(Request) + SkAlign4(nameLen)]; The SkResourceCache is going to try to 'delete' this instead of 'delete []' this. Need different memory management to make this work.
https://codereview.chromium.org/1683883002/diff/40001/src/ports/SkFontHost_fo... File src/ports/SkFontHost_fontconfig.cpp (right): https://codereview.chromium.org/1683883002/diff/40001/src/ports/SkFontHost_fo... src/ports/SkFontHost_fontconfig.cpp:96: char* storage = new char [sizeof(Request) + SkAlign4(nameLen)]; On 2016/02/11 18:23:36, bungeman1 wrote: > The SkResourceCache is going to try to 'delete' this instead of 'delete []' > this. Need different memory management to make this work. Done.
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
On 2016/02/12 22:58:24, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, > http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...) I cannot believe that something as trivial as std::align is so buggy or missing for so long. It is a nice way to allow the user to sidestep all the potential undefined behavior in this area and leave it up to the standard library what is allowed and what isn't. For example arithmetic on uintptr_t or sizeof(uintptr_t) == sizeof(size_t) isn't guaranteed by the standard, and while we can add static_asserts and whatnot, it's nice to just use the thing that does the thing you want (and even has the right name). I'll update this soon.
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/100001
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This should now be ready for review. This fixes an immediate need of Chromium, and could also be an example of how to go about creating a caching SkFontMgr wrapper.
confirmed this fixes the issue. I'd defer real code review to skia experts.
https://codereview.chromium.org/1683883002/diff/120001/src/core/SkResourceCac... File src/core/SkResourceCache.h (right): https://codereview.chromium.org/1683883002/diff/120001/src/core/SkResourceCac... src/core/SkResourceCache.h:39: size_t length() { size_t size() const { ...
with discardable memory, it is harder to cache small things. How wil this get purged?
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/140001
> with discardable memory, it is harder to cache small things. How wil this > get purged? So the setup at the moment is that this uses its own SkResourceCache instance and limits the size to 4kb and does not currently have hooks to purge anything. It is also not using discardable memory anywhere (the thing being held onto is a pointer/reference). This seems acceptable for the moment because 1. Chromium needs this workaround, like, a year ago. 2. The previous patch sets have a rather weaker cache mechanism which is also not purgeable, and we were willing to go with those. 3. We're already having issues with purging things, we're going to need a real global broadcast 'need to purge'. Note that this is also the only user of the static methods of SkTypefaceCache, everything else uses its own instance (so that it can downcast what it gets out). None of those instances currently respond to purge requests either. 4. This is a really small cache. I can certainly work on purgeability (and the rather touchy interface to SkResourceCache::Key), but I'd like to land this first, as this is already complex enough and rather small. https://codereview.chromium.org/1683883002/diff/120001/src/core/SkResourceCac... File src/core/SkResourceCache.h (right): https://codereview.chromium.org/1683883002/diff/120001/src/core/SkResourceCac... src/core/SkResourceCache.h:39: size_t length() { On 2016/02/17 14:12:52, reed1 wrote: > size_t size() const { ... Heh, yes, http://stackoverflow.com/questions/300522/count-vs-length-vs-size-in-a-collec... . I agree that this should be 'size', and I'm gonna change it, but the parameter to 'init' that this is effectively mirroring is named 'length' (and it isn't immediately clear which length it's referring to). I'll change that too then just for sanity. If only the STL had promoted a consistent naming convention.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm w/ const-nit thanks for the explanation about a separate instance. https://codereview.chromium.org/1683883002/diff/140001/src/core/SkResourceCac... File src/core/SkResourceCache.h (right): https://codereview.chromium.org/1683883002/diff/140001/src/core/SkResourceCac... src/core/SkResourceCache.h:41: size_t size() { const?
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ==========
https://codereview.chromium.org/1683883002/diff/140001/src/core/SkResourceCac... File src/core/SkResourceCache.h (right): https://codereview.chromium.org/1683883002/diff/140001/src/core/SkResourceCac... src/core/SkResourceCache.h:41: size_t size() { On 2016/02/17 17:13:39, reed1 wrote: > const? Done.
The CQ bit was checked by bungeman@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org, reed@google.com Link to the patchset: https://codereview.chromium.org/1683883002/#ps160001 (title: "Make method 'const'.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683883002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683883002/160001
Message was sent while issue was closed.
Description was changed from ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 ========== to ========== Add request cache to SkFontHost_fontconfig. The current code deduplicates SkTypeface instances as all font lookups should for better use of the glyph cache. This adds a request cache as well, so that repeated recent requests will return the cached result instead of doing a full lookup. BUG=chromium:424082, chromium:444894 Committed: https://skia.googlesource.com/skia/+/70bb80802198b7fedee58f79d0d68d5f8aba8b62 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://skia.googlesource.com/skia/+/70bb80802198b7fedee58f79d0d68d5f8aba8b62
Message was sent while issue was closed.
You're my hero! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
