837542 - Tab animation is using a lot of memory of gradient pattern cache (windows, HW accel on)

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Avi Halachmi (:avih)]

After opening 4 tabs (of about:blank) in a row, and then closing them, memory spikes temporarily by about 80 MB. After some time (15s - 30s), it seems most of the memory is released. When I have 10 tabs open and then open/close another tab 20 times, the spike is of about 500 MB.

After the same sequence, but with HW accel off (or with the gradients pattern cache disabled at the code), the spike is of about 1MB, which makes it quite clear that the big spike is unrelated to the document, but rather to the gradient patterns cache.

While the spike is not necessarily hurting, and it does go away after a while, we might still be able to use less temporary memory without hurting performance.

This could be the result of several factors:

1. There are a lot of pattern-cache misses during tab animation. This is probably due to the fact that the cell-size is part of the cache key, and the cell size changes with each animation frame, and also, even on the next animation we're not guaranteed to have more cache-hits since the size is non-integer, and depends on the timestamp at which the frame needs to be painted.

2. There are 5 gradient paints (which evaluate to 5 different pattern cache keys) on each animation frame (2 for the tab, 3 for the newtab-button - see bug 837535). Multiply that by about 15 animation frames, and we have created a lot of pattern cache items which will be never used to save memory and will instead just pile (at least for the resizing tabs).

3. Possibly the gradient pattern cache item is holding too much data for the tab animation case. On my system, the tab (and the newtab-button) are 29px tall. Theoretically, we need a surface of 29 pixels to be able to render this background. BenWa mentioned that with D2D, each linear cache item uses 8KB data (1px x 2048px x 4Bpp).

As for the issue of cache misses, bug 835284 (V1 part 2) changes the cell-size to be 1px wide (or tall) for vertical/horizontal linear gradients, which results in 100% cache hits during tab animation (and practically eliminating the big memory spike, even when the raster cache which this bug adds is disabled - but while keeping the 1px cell size change).

Comment 1

[Jeff Muizelaar [:jrmuizel]]

(In reply to Avi Halachmi (:avih) from comment #0)
> 3. Possibly the gradient pattern cache item is holding too much data for the
> tab animation case. On my system, the tab (and the newtab-button) are 29px
> tall. Theoretically, we need a surface of 29 pixels to be able to render
> this background. BenWa mentioned that with D2D, each linear cache item uses
> 8KB data (1px x 2048px x 4Bpp).

I wouldn't be surprised if this is actually higher. I believe the texture size may actually be 4096px * 4Bpp * 1px * 2 (one for gpu memory and one for system memory) So the texture size could easily be 32KB. Process Explorer will show you the amount of gpu memory used. It should be possible for us to count the number of entries in the cache to get an idea of how close 32KB is to the amount that we expect. The 1px height may also be rounded up by the driver to 4px. Which would give us 128KB per cache entry.

Comment 2

[Nicholas Nethercote [inactive]]

This sounds *terrible*.  jrmuizel, are you able to investigate more?

Comment 3

[Avi Halachmi (:avih)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> ...
> Which would give us 128KB per cache entry.

This explains a lot and gets to the same magnitude as my observation, but still short in half :). 128K * 15 frames * 2 new cache items per frame (the newtab button doesn't get cache misses) = 3.8M, while I'm seeing about 8M per tab animation.

Also worth noting that my system uses an HD4000m iGPU in an optimus configuration, and the same magnitude of mem spike was also observed by Jeff and Vladan on another system with HD4000, but without optimus.

Another way to reproduce this issue is having 10 tabs open, and resizing the window horizontally such that all tabs shrink/expand. I can add few hundreds MB of ram usage after few seconds of such resizing.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> 
> I wouldn't be surprised if this is actually higher. I believe the texture
> size may actually be 4096px * 4Bpp * 1px * 2 (one for gpu memory and one for
> system memory) So the texture size could easily be 32KB. Process Explorer
> will show you the amount of gpu memory used. It should be possible for us to
> count the number of entries in the cache to get an idea of how close 32KB is
> to the amount that we expect. The 1px height may also be rounded up by the
> driver to 4px. Which would give us 128KB per cache entry.

It looks the intel hardware requires vertical alignment of at least 2 and not 4.
(See the Alignment Unit Size section of "Volume 1 Part 1: Graphics Core")

Comment 5

[Jet Villegas (inactive)]

Adding 838886 as a dependency if putting a cache ceiling will help here...

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Bug 838758 will have largely fixed this.

Comment 7

[Avi Halachmi (:avih)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> Bug 838758 will have largely fixed this.

It does fix the specific case of this bug. We also have bug 838886 to limit the gradients cache.

Should we also file a bug about single cache item which uses a lot of memory (e.g. on Intel/HD4000)? or do we let it off as a driver issue?

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Closing this because there's no more work to do other than bug 838886 and bug 848742 (the intel bug)

Comment 9

[Nicholas Nethercote [inactive]]

> > Bug 838758 will have largely fixed this.

Oh, I missed that.  Great.  Should that fix be ported to Aurora and/or Beta?

Comment 10

[Jeff Muizelaar [:jrmuizel]]

I've asked for Aurora approval. I think it's too risky for Beta.