Commit ac5d8bee authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Limit HashMap growth in CSSSegmentedFontFace by moving to LruCache

Addresses unbounded memory growth of font_data_table_ in
CSSSegmentedFontFace leading to out of memory situations when animating
variable fonts.

Manually tested observing Chrome's task manager memory consumption on
Linux for a tab running a variable font animation test page. Tab memory
stays around 75MB over a period of 2 minutes vs. growing to around 1.3GB
before.

Bug: 778352
Change-Id: I53b1f5805974d667aec35d6a9358205f5346b63c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032112
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740277}
parent bd741691
...@@ -33,11 +33,22 @@ ...@@ -33,11 +33,22 @@
#include "third_party/blink/renderer/platform/fonts/segmented_font_data.h" #include "third_party/blink/renderer/platform/fonts/segmented_font_data.h"
#include "third_party/blink/renderer/platform/fonts/simple_font_data.h" #include "third_party/blink/renderer/platform/fonts/simple_font_data.h"
// See comment below in CSSSegmentedFontFace::GetFontData - the cache from
// CSSSegmentedFontFace (which represents a group of @font-face declarations
// with identical FontSelectionCapabilities but differing by unicode-range) to
// FontData/SegmentedFontData, (i.e. the actual font blobs that can be used for
// shaping and painting retrieved from a CSSFontFaceSource) is usually small
// (less than a dozen, up to tens) for non-animation-cases, but grows fast to
// thousands when animating variable font parameters. Set a limit until we start
// dropping cache entries in animation scenarios.
static constexpr size_t kFontDataTableMaxSize = 250;
namespace blink { namespace blink {
CSSSegmentedFontFace::CSSSegmentedFontFace( CSSSegmentedFontFace::CSSSegmentedFontFace(
FontSelectionCapabilities font_selection_capabilities) FontSelectionCapabilities font_selection_capabilities)
: font_selection_capabilities_(font_selection_capabilities), : font_selection_capabilities_(font_selection_capabilities),
font_data_table_(kFontDataTableMaxSize),
first_non_css_connected_face_(font_faces_.end()), first_non_css_connected_face_(font_faces_.end()),
approximate_character_count_(0) {} approximate_character_count_(0) {}
...@@ -45,10 +56,10 @@ CSSSegmentedFontFace::~CSSSegmentedFontFace() = default; ...@@ -45,10 +56,10 @@ CSSSegmentedFontFace::~CSSSegmentedFontFace() = default;
void CSSSegmentedFontFace::PruneTable() { void CSSSegmentedFontFace::PruneTable() {
// Make sure the glyph page tree prunes out all uses of this custom font. // Make sure the glyph page tree prunes out all uses of this custom font.
if (font_data_table_.IsEmpty()) if (!font_data_table_.size())
return; return;
font_data_table_.clear(); font_data_table_.Clear();
} }
bool CSSSegmentedFontFace::IsValid() const { bool CSSSegmentedFontFace::IsValid() const {
...@@ -102,16 +113,26 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData( ...@@ -102,16 +113,26 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData(
FontCacheKey key = font_description.CacheKey( FontCacheKey key = font_description.CacheKey(
FontFaceCreationParams(), is_unique_match, font_selection_request); FontFaceCreationParams(), is_unique_match, font_selection_request);
scoped_refptr<SegmentedFontData>& font_data = // font_data_table_ caches FontData and SegmentedFontData instances, which
font_data_table_.insert(key, nullptr).stored_value->value; // provide SimpleFontData objects containing FontPlatformData objects. In the
if (font_data && font_data->NumFaces()) { // case of variable font animations, the variable instance SkTypeface is
// No release, we have a reference to an object in the cache which should // contained in these FontPlatformData objects. In other words, this cache
// retain the ref count it has. // stores the recently used variable font instances during a variable font
return font_data; // animation. The cache reflects in how many different sizes, synthetic styles
} // (bold / italic synthetic versions), or for variable fonts, in how many
// variable instances (stretch/style/weightand font-variation-setings
if (!font_data) // variations) the font is instantiated. In non animation scenarios, there is
font_data = SegmentedFontData::Create(); // usually only a small number of FontData/SegmentedFontData instances created
// per CSSSegmentedFontFace. Whereas in variable font animations, this number
// grows rapidly.
scoped_refptr<SegmentedFontData>* cached_font_data =
font_data_table_.Get(key);
if (cached_font_data && (*cached_font_data) &&
(*cached_font_data)->NumFaces())
return *cached_font_data;
scoped_refptr<SegmentedFontData> created_font_data;
created_font_data = SegmentedFontData::Create();
FontDescription requested_font_description(font_description); FontDescription requested_font_description(font_description);
if (!font_selection_capabilities_.HasRange()) { if (!font_selection_capabilities_.HasRange()) {
...@@ -131,18 +152,21 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData( ...@@ -131,18 +152,21 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData(
(*it)->CssFontFace()->GetFontData(requested_font_description)) { (*it)->CssFontFace()->GetFontData(requested_font_description)) {
DCHECK(!face_font_data->IsSegmented()); DCHECK(!face_font_data->IsSegmented());
if (face_font_data->IsCustomFont()) { if (face_font_data->IsCustomFont()) {
font_data->AppendFace(base::AdoptRef(new FontDataForRangeSet( created_font_data->AppendFace(base::AdoptRef(new FontDataForRangeSet(
std::move(face_font_data), (*it)->CssFontFace()->Ranges()))); std::move(face_font_data), (*it)->CssFontFace()->Ranges())));
} else { } else {
font_data->AppendFace(base::AdoptRef(new FontDataForRangeSetFromCache( created_font_data->AppendFace(
std::move(face_font_data), (*it)->CssFontFace()->Ranges()))); base::AdoptRef(new FontDataForRangeSetFromCache(
std::move(face_font_data), (*it)->CssFontFace()->Ranges())));
} }
} }
} }
if (font_data->NumFaces()) { if (created_font_data->NumFaces()) {
scoped_refptr<SegmentedFontData> put_to_cache(created_font_data);
font_data_table_.Put(std::move(key), std::move(put_to_cache));
// No release, we have a reference to an object in the cache which should // No release, we have a reference to an object in the cache which should
// retain the ref count it has. // retain the ref count it has.
return font_data; return created_font_data;
} }
return nullptr; return nullptr;
......
...@@ -31,7 +31,7 @@ ...@@ -31,7 +31,7 @@
#include "third_party/blink/renderer/platform/fonts/segmented_font_data.h" #include "third_party/blink/renderer/platform/fonts/segmented_font_data.h"
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/wtf/forward.h" #include "third_party/blink/renderer/platform/wtf/forward.h"
#include "third_party/blink/renderer/platform/wtf/hash_map.h" #include "third_party/blink/renderer/platform/wtf/lru_cache.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/vector.h" #include "third_party/blink/renderer/platform/wtf/vector.h"
...@@ -79,7 +79,10 @@ class CSSSegmentedFontFace final ...@@ -79,7 +79,10 @@ class CSSSegmentedFontFace final
using FontFaceList = HeapListHashSet<Member<FontFace>>; using FontFaceList = HeapListHashSet<Member<FontFace>>;
FontSelectionCapabilities font_selection_capabilities_; FontSelectionCapabilities font_selection_capabilities_;
HashMap<FontCacheKey, scoped_refptr<SegmentedFontData>> font_data_table_;
WTF::LruCache<FontCacheKey, scoped_refptr<SegmentedFontData>>
font_data_table_;
// All non-CSS-connected FontFaces are stored after the CSS-connected ones. // All non-CSS-connected FontFaces are stored after the CSS-connected ones.
FontFaceList font_faces_; FontFaceList font_faces_;
FontFaceList::iterator first_non_css_connected_face_; FontFaceList::iterator first_non_css_connected_face_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment