Regression(r177786): GlyphMetricsMap<T>::locatePageSlowCase() fills existing pages with unknown metrics

[Chris Dumez]

GlyphMetricsMap<T>::locatePageSlowCase() fills existing pages with unknown metrics after <http://trac.webkit.org/changeset/177786>. Previously, we would only to this for new pages.

[Chris Dumez]

Created attachment 279040 [details]
Patch

[Chris Dumez]

Still testing locally.

[Chris Dumez]

Found this by looking at PLT profiles that seeing that WebCore::GlyphMetricsMap<WebCore::FloatRect>::locatePageSlowCase(unsigned int) is slow.

[Antti Koivisto]

Comment on attachment 279040 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279040&action=review

Nice find, r=me

> Source/WebCore/platform/graphics/GlyphMetricsMap.h:123
> +    auto& pageInMap = m_pages->add(pageNumber, nullptr).iterator->value;
> +    if (!pageInMap)
> +        pageInMap = std::make_unique<GlyphMetricsPage>(unknownMetrics());
> +    return *pageInMap;

If you want to be fancy you could do something like

return *m_pages.ensure(pageNumber, [] { 
   return std::make_unique<GlyphMetricsPage>(unknownMetrics(); }
).iterator->value;

[WebKit Commit Bot]

Comment on attachment 279040 [details]
Patch

Clearing flags on attachment: 279040

Committed r201023: <http://trac.webkit.org/changeset/201023>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Looks like a ~1% PLT progression on Mac.

[Chris Dumez]

(In reply to comment #7)
> Looks like a ~1% PLT progression on Mac.

Ditto on iOS.