fonts: Cap the max number of entries in server side glyph cache tracking
The SkStrikeServer maintains a map of SkGlyphCacheState to track the
cache for a strike on the client. This entry is evicted only on a
failure to lock, which means that the map can potentially grow unbounded
if entries are not reused after eviction.
Make sure we check that they are deleted after some limit.
R=herb@google.com
Bug:878966
Change-Id: I4adb06c35661049328f6e0bde52fb1c774d0c29b
Reviewed-on: https://skia-review.googlesource.com/150443
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>
diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index 4fe2f42..4a8d917 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -515,9 +515,23 @@
fLockedDescs.insert(keyDescPtr);
fRemoteGlyphStateMap[keyDescPtr] = std::move(cacheState);
cacheStatePtr->ensureScalerContext(paint, props, matrix, flags, effects);
+
+ checkForDeletedEntries();
return cacheStatePtr;
}
+void SkStrikeServer::checkForDeletedEntries() {
+ auto it = fRemoteGlyphStateMap.begin();
+ while (fRemoteGlyphStateMap.size() > fMaxEntriesInDescriptorMap &&
+ it != fRemoteGlyphStateMap.end()) {
+ if (fDiscardableHandleManager->isHandleDeleted(it->second->discardableHandleId())) {
+ it = fRemoteGlyphStateMap.erase(it);
+ } else {
+ ++it;
+ }
+ }
+}
+
// -- SkGlyphCacheState ----------------------------------------------------------------------------
SkStrikeServer::SkGlyphCacheState::SkGlyphCacheState(
std::unique_ptr<SkDescriptor> keyDescriptor, uint32_t discardableHandleId)
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index d9abb7c..2faf64d 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -105,9 +105,10 @@
// allowed.
virtual bool lockHandle(SkDiscardableHandleId) = 0;
- // TODO(khushalsagar): Add an API which checks whether a handle is still
- // valid without locking, so we can avoid tracking stale handles once they
- // have been purged on the remote side.
+ // Returns true if a handle has been deleted on the remote client. It is
+ // invalid to use a handle id again with this manager once this returns true.
+ // TODO(khushalsagar): Make pure virtual once chrome implementation lands.
+ virtual bool isHandleDeleted(SkDiscardableHandleId) { return false; }
};
SkStrikeServer(DiscardableHandleManager* discardableHandleManager);
@@ -128,10 +129,20 @@
SkScalerContextFlags flags,
SkScalerContextEffects* effects);
+ void setMaxEntriesInDescriptorMapForTesting(size_t count) {
+ fMaxEntriesInDescriptorMap = count;
+ }
+ size_t remoteGlyphStateMapSizeForTesting() const { return fRemoteGlyphStateMap.size(); }
+
private:
+ static constexpr size_t kMaxEntriesInDescriptorMap = 2000u;
+
+ void checkForDeletedEntries();
+
SkDescriptorMap<std::unique_ptr<SkGlyphCacheState>> fRemoteGlyphStateMap;
DiscardableHandleManager* const fDiscardableHandleManager;
SkTHashSet<SkFontID> fCachedTypefaces;
+ size_t fMaxEntriesInDescriptorMap = kMaxEntriesInDescriptorMap;
// State cached until the next serialization.
SkDescriptorSet fLockedDescs;
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index 24ede91..1493f2b 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -41,6 +41,7 @@
// Client implementation.
bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
void notifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
+ bool isHandleDeleted(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
void unlockAll() { fLockedHandles.reset(); }
void unlockAndDeleteAll() {
@@ -342,6 +343,50 @@
discardableManager->unlockAndDeleteAll();
}
+DEF_TEST(SkRemoteGlyphCache_PurgesServerEntries, reporter) {
+ sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
+ SkStrikeServer server(discardableManager.get());
+ server.setMaxEntriesInDescriptorMapForTesting(1u);
+ SkStrikeClient client(discardableManager, false);
+
+ {
+ auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
+ int glyphCount = 10;
+ auto serverBlob = buildTextBlob(serverTf, glyphCount);
+
+ const SkSurfaceProps props(SkSurfaceProps::kLegacyFontHost_InitType);
+ SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server);
+ SkPaint paint;
+ REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 0u);
+ cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
+ REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+ }
+
+ // Serialize to release the lock from the strike server and delete all current
+ // handles.
+ std::vector<uint8_t> fontData;
+ server.writeStrikeData(&fontData);
+ discardableManager->unlockAndDeleteAll();
+
+ // Use a different typeface. Creating a new strike should evict the previous
+ // one.
+ {
+ auto serverTf = SkTypeface::MakeFromName("Georgia", SkFontStyle());
+ int glyphCount = 10;
+ auto serverBlob = buildTextBlob(serverTf, glyphCount);
+
+ const SkSurfaceProps props(SkSurfaceProps::kLegacyFontHost_InitType);
+ SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server);
+ SkPaint paint;
+ REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+ cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
+ REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+ }
+
+ // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
+ discardableManager->unlockAndDeleteAll();
+}
+
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsPath, reporter, ctxInfo) {
sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
SkStrikeServer server(discardableManager.get());