Bug 228224 – [GPU Process] Start tracking resource uses for NativeImages and Fonts

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[GPU Process] Start tracking resource uses for NativeImages and Fonts

[Myles C. Maxfield]

Created attachment 434070 [details]
Patch for reviewing

[Myles C. Maxfield]

Created attachment 434071 [details]
Patch for EWS

[Myles C. Maxfield]

Created attachment 434267 [details]
Patch

[Myles C. Maxfield]

Created attachment 434309 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 434309 [details]
Patch

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

> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:105
> +    if (delegate) {

Maybe an early return will look clearer

if (!delegate)
    return std::nullopt;

> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:108
> +        if (changeFlags.contains(GraphicsContextState::StrokePatternChange))
> +            delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);

Can't this be written like this:

if (strokePatternRenderingResourceIdentifier)
    delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);

> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:81
>  void RemoteResourceCacheProxy::cacheNativeImage(NativeImage& image)

Maybe we need to choose new names for these functions since they do more than just caching the resources:

cacheNativeImageAndUse()
ensureNativeImageIsCachedAndUse()
recordNativeImageUse()

Or something similar. This may be also done in a separate patch.

[Myles C. Maxfield]

Comment on attachment 434309 [details]
Patch

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

>> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:108
>> +            delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);
> 
> Can't this be written like this:
> 
> if (strokePatternRenderingResourceIdentifier)
>     delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);

We can! Good catch.

>> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:81
>>  void RemoteResourceCacheProxy::cacheNativeImage(NativeImage& image)
> 
> Maybe we need to choose new names for these functions since they do more than just caching the resources:
> 
> cacheNativeImageAndUse()
> ensureNativeImageIsCachedAndUse()
> recordNativeImageUse()
> 
> Or something similar. This may be also done in a separate patch.

Yeah, this is a good idea. I'll do this in a follow-up patch.

[Myles C. Maxfield]

Committed r280356 (240003@main): <https://commits.webkit.org/240003@main>

[Radar WebKit Bug Importer]

<rdar://problem/81186859>

[Myles C. Maxfield]

Comment on attachment 434309 [details]
Patch

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

>>> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:81
>>>  void RemoteResourceCacheProxy::cacheNativeImage(NativeImage& image)
>> 
>> Maybe we need to choose new names for these functions since they do more than just caching the resources:
>> 
>> cacheNativeImageAndUse()
>> ensureNativeImageIsCachedAndUse()
>> recordNativeImageUse()
>> 
>> Or something similar. This may be also done in a separate patch.
> 
> Yeah, this is a good idea. I'll do this in a follow-up patch.

https://bugs.webkit.org/show_bug.cgi?id=228495