Bug 77573 – [Chromium] Purge Skia's GPU texture cache using GL_CHROMIUM_gpu_memory_manager

• Component: Canvas
• Status: RESOLVED DUPLICATE

[Jeff Timanus]

The context used to render Canvas2D content is allocated as a static object in SharedGraphicsContext.  This context will allocate up to 96 MB of texture memory for caching.  This cache is not purged when tabs are no longer visible.

This bug is for tracking the integration between the GL_CHROMIUM_gpu_memory_manager [1] extension, and Ganesh.

The plan for this work is that the extension will call-back to the GrContext when all WebViews within a given render process have been hidden, and inform the GrContext to purge all of its cached GPU resources.

[1] https://bugs.webkit.org/show_bug.cgi?id=77155

[Jeff Timanus]

Created attachment 125141 [details]
Patch

[Jeff Timanus]

(In reply to comment #1)
> Created an attachment (id=125141) [details]
> Patch

This patch implements a prototype of the integration required plumb the Ganesh context through to the GL_CHROMIUM_gpu_memory_manager extension.

The implementation is not quite complete, as I'm waiting for the extension in issue 77155 to land.

At this point, I'd appreciate some feedback on the approach taken to connect the WebView to the GrContext used by Ganesh.  

During WebViewImpl::composite, I unregister all of the affecting contexts, and then during Canvas2DLayerChromium::pushPropertiesTo, the GraphicsContext3D is registered with the WebViewImpl via the CCLayerTreeHostClient.  All of the collected contexts are then to be registered with the extension.

[Nat Duca]

Comment on attachment 125141 [details]
Patch

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

> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:85
> +namespace {

I assume this is going to be part of Michal's patch, not yours?

> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:158
> +    if (extensions3d->supports("gpu_memory_management_CHROMIUM")) {

I dont think this is right... the person owning the context should be installing the extension. I think Michal has a patch that makes Extensions3DChromium support an allocation callback

> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:116
> +void WebLayerTreeViewImpl::registerAffectingContext(WebCore::GraphicsContext3D* context)

Lets use the same words we're using on the extension. addAffectedContext/etc...

> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:118
> +}

i assume this is going to make the same call up to m_client (which is WebViewImpl?)

> Source/WebKit/chromium/src/WebViewImpl.cpp:1267
> +#if USE(SKIA)

Lets make this unconditional on skia. Its a good concept to have anyways.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1289
> +

So this composite function isn't called in threaded mode.

Lets add two methods in CCLayerTreeHostClient called
 willCommit
 didCommit

CCSingleThreadProxy should call that in its commitIfneeded code

CCThreadProxy should too

[Jeff Timanus]

Comment on attachment 125141 [details]
Patch

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

After offline discussion with mmocny@ and nduca@, some changes are required.  See inline comments.

> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:160
> +    }

Constructing a callback for every context is unnecessary.  A callback is only needed for contexts with a GrContext.  This code path should be moved to GraphicsContext3DPrivate::GrContext().

> Source/WebKit/chromium/src/WebViewImpl.cpp:1276
> +

Resetting all of the affected widgets during every composite is unnecessarily expensive.  Instead, the set deltas between successive composite operations need to be processed.  This will minimize the number of calls into the extension.

[Jeff Timanus]

Created attachment 125396 [details]
Patch

[Jeff Timanus]

Comment on attachment 125141 [details]
Patch

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

>> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:85
>> +namespace {
> 
> I assume this is going to be part of Michal's patch, not yours?

Michal's patch introduces the interface, but my patch will still need the implementation that purges Ganesh's texture cache.

>> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:158
>> +    if (extensions3d->supports("gpu_memory_management_CHROMIUM")) {
> 
> I dont think this is right... the person owning the context should be installing the extension. I think Michal has a patch that makes Extensions3DChromium support an allocation callback

I moved the installation of the extension to the shared graphics context.  It's safe to assume that a shared graphics context will be associated with a GrContext.

>> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:116
>> +void WebLayerTreeViewImpl::registerAffectingContext(WebCore::GraphicsContext3D* context)
> 
> Lets use the same words we're using on the extension. addAffectedContext/etc...

Changed to addAffectingContext.  The extension is for adding affected web-views.

>> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:118
>> +}
> 
> i assume this is going to make the same call up to m_client (which is WebViewImpl?)

I'm uncertain how this class should interact with the extension.  m_client is not a WebViewImpl.  It is a WebLayerTreeViewClient, which is implemented by the Compositor class (ui/gfx/compositor/compositor.h)

>> Source/WebKit/chromium/src/WebViewImpl.cpp:1289
>> +
> 
> So this composite function isn't called in threaded mode.
> 
> Lets add two methods in CCLayerTreeHostClient called
>  willCommit
>  didCommit
> 
> CCSingleThreadProxy should call that in its commitIfneeded code
> 
> CCThreadProxy should too

Done.  CCThreadProxy does not have a commitIfNeeded routine.  Instead I added it to the compositeAndReadback(...) routine.  Is this safe?

[Jeff Timanus]

From an offline discussion with bsalomon, we decided that it would be beneficial if this patch set the texture cache size to 0 upon background notification.

If the cache is only purged to 0 when backgrounded, then the GPU memory footprint could still increase as non-raf rendering events are fired.

This change will require the GL_CHROMIUM_gpu_memory_manager to provide foregrounded notifications as well as backgrounded.

[Michal Mocny]

*** This bug has been marked as a duplicate of bug 78265 ***