Bug 223082

Summary: Shutdown ANGLE on GPU process when last WebGL context is destroyed
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, darin, dino, ews-watchlist, graouts, gyuyoung.kim, jonlee, kbr, kkinnunen, kondapallykalyan, ryuan.choi, sergio, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 217211    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Kimmo Kinnunen 2021-03-11 12:05:13 PST 
Shutdown ANGLE on GPU process when last WebGL context is destroyed
Comment 1 Kimmo Kinnunen 2021-03-12 07:23:24 PST 
Created attachment 423046 [details]
Patch
Comment 2 Kimmo Kinnunen 2021-03-12 07:27:59 PST 
Created attachment 423047 [details]
Patch
Comment 3 Kimmo Kinnunen 2021-03-12 07:59:17 PST 
Created attachment 423050 [details]
Patch
Comment 4 Kimmo Kinnunen 2021-03-12 10:42:55 PST 
Created attachment 423062 [details]
Patch
Comment 5 Kenneth Russell 2021-03-12 17:17:02 PST 
Comment on attachment 423062 [details]
Patch

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

Looks good. The retain/release code looks correct to me. A few small points; r+ though.

> Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLEEGLUtilities.h:2
> + * Copyright (C) 2020 Apple Inc. All rights reserved.

2021?

> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:55
> +static constexpr Seconds dispatchReleaseAllResourcesIfUnusedTimeout = 0.2_s;

This feels like a short timeout to me. Consider bumping it up, maybe even to 1s or so.

> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:56
> +static unsigned remoteGraphicsContextCount;

Guaranteed to be zero-initialized?
Comment 6 Darin Adler 2021-03-12 17:44:59 PST 
Comment on attachment 423062 [details]
Patch

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

>> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:56
>> +static unsigned remoteGraphicsContextCount;
> 
> Guaranteed to be zero-initialized?

Yes, globals like this are guaranteed to be zero-initialized by the C and C++ standards.
Comment 7 Yusuke Suzuki 2021-03-12 21:44:20 PST 
Internal memory benchmark observed 0.8-1.5% progression with this change.
Comment 8 Radar WebKit Bug Importer 2021-03-15 12:59:30 PDT 
<rdar://problem/75442792>
Comment 9 EWS 2021-03-17 04:47:52 PDT 
Committed r274557: <https://commits.webkit.org/r274557>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423062 [details].