Bug 228224

Summary: [GPU Process] Start tracking resource uses for NativeImages and Fonts
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, jonlee, sabouhallawa, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 228222    
Bug Blocks: 228216, 228495    
Attachments:
Description Flags
Patch for reviewing
none
Patch for EWS
none
Patch
none
Patch sabouhallawa: review+, ews-feeder: commit-queue-

Description Myles C. Maxfield 2021-07-23 00:40:26 PDT 
[GPU Process] Start tracking resource uses for NativeImages and Fonts
Comment 1 Myles C. Maxfield 2021-07-23 00:45:09 PDT 
Created attachment 434070 [details]
Patch for reviewing
Comment 2 Myles C. Maxfield 2021-07-23 00:46:41 PDT 
Created attachment 434071 [details]
Patch for EWS
Comment 3 Myles C. Maxfield 2021-07-26 21:04:36 PDT 
Created attachment 434267 [details]
Patch
Comment 4 Myles C. Maxfield 2021-07-27 13:34:09 PDT 
Created attachment 434309 [details]
Patch
Comment 5 Said Abou-Hallawa 2021-07-27 13:52:30 PDT 
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.
Comment 6 Myles C. Maxfield 2021-07-27 15:51:59 PDT 
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.
Comment 7 Myles C. Maxfield 2021-07-27 15:53:05 PDT 
Committed r280356 (240003@main): <https://commits.webkit.org/240003@main>
Comment 8 Radar WebKit Bug Importer 2021-07-27 15:54:26 PDT 
<rdar://problem/81186859>
Comment 9 Myles C. Maxfield 2021-07-27 16:09:04 PDT 
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