Bug 190312 – Prewarm FontDatabase on process swap

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Antti Koivisto]

Prewarm process using cached information about the fonts the page likely needs.

[Antti Koivisto]

Created attachment 351675 [details]
patch

[Alex Christensen]

Comment on attachment 351675 [details]
patch

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

> Source/WebCore/page/PrewarmInformation.h:52
> +        return { };

std::nullopt

> Source/WebKit/UIProcess/WebProcessPool.h:702
> +    HashMap<String, std::unique_ptr<WebCore::PrewarmInformation>> m_prewarmInformationForRegistrableDomain;

It seems like the map should not have unique_ptr values.  Why not HashMap<String, WebCore::PrewarmInformation>?

> Source/WebKit/WebProcess/WebProcess.h:252
>      void prewarm();
> +    void prewarmWithDomainInformation(const WebCore::PrewarmInformation&);

Do we need both of these?

[Antti Koivisto]

> It seems like the map should not have unique_ptr values.  Why not
> HashMap<String, WebCore::PrewarmInformation>?

Complex structures as map values are a bad idea as rehashing can involve lots of copying. PrewarmInformation is not that complex yet but the idea is for it to be expandable.

> > Source/WebKit/WebProcess/WebProcess.h:252
> >      void prewarm();
> > +    void prewarmWithDomainInformation(const WebCore::PrewarmInformation&);
> 
> Do we need both of these?

First one is for general prewarming that doesn't require any context and is done right after process creation. The seconds one requires we know the domain and happens on the first page load in the process.

Some renaming might be good to clarify this.

[Alex Christensen]

Comment on attachment 351675 [details]
patch

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

> Source/WebKit/UIProcess/WebProcessPool.cpp:2268
> +    auto& value = m_prewarmInformationForRegistrableDomain.ensure(registrableDomain, [] {
> +        return std::make_unique<WebCore::PrewarmInformation>();
> +    }).iterator->value;
> +
> +    *value = prewarmInformation;

I agree with using std::unique_ptr for the key now, but could we call set instead of ensure, creating an empty structure, then copying values into it?

[Antti Koivisto]

> I agree with using std::unique_ptr for the key now, but could we call set
> instead of ensure, creating an empty structure, then copying values into it?

I don't see how that would be better.

[Antti Koivisto]

Created attachment 351764 [details]
patch

[Antti Koivisto]

> std::nullopt

Why?

[Antti Koivisto]

Created attachment 351766 [details]
patch

[Antti Koivisto]

Created attachment 351768 [details]
patch

[Chris Dumez]

Comment on attachment 351768 [details]
patch

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

r=me

> Source/WebKit/UIProcess/WebProcessPool.h:702
> +    HashMap<String, std::unique_ptr<WebCore::PrewarmInformation>> m_prewarmInformationForRegistrableDomain;

It seems we use PerRegistrableDomain naming above, let's be consistent?

[Antti Koivisto]

Created attachment 351869 [details]
patch

[WebKit Commit Bot]

Comment on attachment 351869 [details]
patch

Clearing flags on attachment: 351869

Committed r236959: <https://trac.webkit.org/changeset/236959>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45120617>