[chromium] Preserve offscreen tiles instead of immediately recycling them
Created attachment 111493 [details] Patch
I went back and forth a lot about whether to move the texture recycling or just remove it, and ended up on the removal end after some discussions with gman and nduca. Thoughts?
Comment on attachment 111493 [details] Patch Yay for changes with more deletions than additions! This looks awesome.
Comment on attachment 111493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111493&action=review This looks great! > Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:-90 > - m_unusedTiles.clear(); can you delete m_unusedTiles from the class definition in the .h as well? It seems unused.
LGTM, I was about to make similar changes. Although I think we will probably need recycle textures in the future, in the TextureManager; I'd be surprised if every driver reacted elegantly to the churn.
(In reply to comment #5) > LGTM, I was about to make similar changes. Although I think we will probably need recycle textures in the future, in the TextureManager; I'd be surprised if every driver reacted elegantly to the churn. Absolutely agreed that the TextureManager is a better place to do this recycling. My preference is to keep the code simple for now and, when we have a good use case that demonstrates a performance problem, use that to tune the right pool size, rather than prematurely doing it now via guesswork.
Committed r97809: <http://trac.webkit.org/changeset/97809>
(In reply to comment #7) > Committed r97809: <http://trac.webkit.org/changeset/97809> I like the cleanup but I think what we may see with this change is that our texture usage keep growing until we hit the texture reclaim limit. At that point, every time we cross a tile boundary we'll be deleting a row of texture tiles and allocating a new one. The good news is that the tiles we'll be deleting will likely be from a part of the page that's pretty far away from our current scroll position. The bad news is that we'll be churning (deleting and recreating) textures a lot. Our texture manager is unfortunately not able to recycle the actual GL texture objects. Given that our tiles are all the same size it would be very convenient if it did. Maybe that's a natural next step?
(In reply to comment #8) > (In reply to comment #7) > > Committed r97809: <http://trac.webkit.org/changeset/97809> > > I like the cleanup but I think what we may see with this change is that our texture usage keep growing until we hit the texture reclaim limit. At that point, every time we cross a tile boundary we'll be deleting a row of texture tiles and allocating a new one. Yeah, having texture usage grow was my intention. Rather than have the layer do this reclamation, this will let the texture manager handle it naturally using LRU. If the active page has nothing else going on, I don't see any problem with texture usage sitting up at the soft limit, because it means that scrolling the entire page will be really fast. > Our texture manager is unfortunately not able to recycle the actual GL texture objects. Given that our tiles are all the same size it would be very convenient if it did. Maybe that's a natural next step? I am not entirely convinced of how much of a problem this is. We are already destroying and recreating tons of textures as layers are destroyed and created or as we switch tab focus. I think there are other tasks that are a higher priority to take care of, like improving the responsiveness of long commits by interleaving input and drawing.