Bug 77573 - [Chromium] Purge Skia's GPU texture cache using GL_CHROMIUM_gpu_memory_manager
Summary: [Chromium] Purge Skia's GPU texture cache using GL_CHROMIUM_gpu_memory_manager
Status: RESOLVED DUPLICATE of bug 78265
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jeff Timanus
URL:
Keywords:
Depends on: 77155
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-01 13:05 PST by Jeff Timanus
Modified: 2012-02-10 06:19 PST (History)
6 users (show)

See Also:


Attachments
Patch (10.16 KB, patch)
2012-02-02 09:01 PST, Jeff Timanus
no flags Details | Formatted Diff | Diff
Patch (12.75 KB, patch)
2012-02-03 13:24 PST, Jeff Timanus
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Timanus 2012-02-01 13:05:28 PST
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
Comment 1 Jeff Timanus 2012-02-02 09:01:26 PST
Created attachment 125141 [details]
Patch
Comment 2 Jeff Timanus 2012-02-02 11:03:42 PST
(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.
Comment 3 Nat Duca 2012-02-02 13:45:55 PST
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
Comment 4 Jeff Timanus 2012-02-02 13:47:23 PST
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.
Comment 5 Jeff Timanus 2012-02-03 13:24:32 PST
Created attachment 125396 [details]
Patch
Comment 6 Jeff Timanus 2012-02-03 13:37:10 PST
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?
Comment 7 Jeff Timanus 2012-02-07 11:42:59 PST
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.
Comment 8 Michal Mocny 2012-02-10 06:19:18 PST

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