We want this for less memory usage. I have a patch that takes the following approach: - Register all of the live ClientTiledLayerBuffers - When a timer ticks call DiscardOldBackBuffers() on all the live ClientTiledLayerBuffers DiscardOldBackBuffers(): if tile.old: DiscardBackBuffer() tile.old = false else: tile.old = true; Flip also marks tile.old = false This means if a tile survives has not used it's back buffer since the last tick it will be discarded.

This is a different design than I originally proposed. I implemented the first one and it was ok. Benoit didn't like it. I implemented this second approach as an alternative. It has some advantages but unfortunately requires careful management of which TileClients are in the ExpirationTracker. I accidentally lost my first implementation.

Comment on attachment 8463689 [details] [diff] [review] Use nsExpiration to expire back buffers Review of attachment 8463689 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/TiledContentClient.cpp @@ +619,5 @@ > } > mBackLock->ReadUnlock(); > mBackBuffer = nullptr; > mBackLock = nullptr; > + gTileExpiry->RemoveObject(this); Everything is on the same thread in TileClient, right? No worries about DiscardBackBuffer getting called by TileExpiry and some "regular" method at the same time?

(In reply to Milan Sreckovic [:milan] from comment #2) > Everything is on the same thread in TileClient, right? No worries about > DiscardBackBuffer getting called by TileExpiry and some "regular" method at > the same time? Yeah, all of the TileCLient stuff happens in the main thread or painting thread if someday we separate the two.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1) > Created attachment 8463689 [details] [diff] [review] > Use nsExpiration to expire back buffers > > This is a different design than I originally proposed. I implemented the > first one and it was ok. Benoit didn't like it. I implemented this second > approach as an alternative. It has some advantages but unfortunately > requires careful management of which TileClients are in the > ExpirationTracker. > > I accidentally lost my first implementation. Could you please re-base your patch for v2.0 ?

Setting this to 2.0+ given it is helping here : https://bugzilla.mozilla.org/show_bug.cgi?id=1038884#c112

Comment on attachment 8464910 [details] [diff] [review] A cleaner version of the safer version Review of attachment 8464910 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/TiledContentClient.cpp @@ +405,5 @@ > > +class TileExpiry MOZ_FINAL : public nsExpirationTracker<TileClient, 3> > +{ > + public: > + TileExpiry() : nsExpirationTracker(1000) {} Indentation is bad. @@ +415,5 @@ > +}; > +TileExpiry *gTileExpiry = new TileExpiry(); > + > +void > +TileClient::WriteMon::Set(TileClient * const aContainer, RefPtr<TextureClient> aNewValue) TileClient* const. I don't think the const really is needed here TBH. Same for ::Set below ::: gfx/layers/client/TiledContentClient.h @@ +232,5 @@ > void DiscardFrontBuffer(); > > void DiscardBackBuffer(); > > + class WriteMon { public: The indentation and spacing is bad. You should have a comment about what the purporse of the WriteMon is. @@ +237,5 @@ > + operator TextureClient*() const {return mBuffer; } > + void Set(TileClient * const container, RefPtr<TextureClient>); > + void Set(TileClient * const container, TextureClient*); > + RefPtr<TextureClient> operator ->() {return mBuffer; } > + bool operator==(const TextureClient* &o) { You need to also overwrite the != operator.

Is the v2.0 patch safe to uplift or does it need cleaning up?

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #12) > Is the v2.0 patch safe to uplift or does it need cleaning up? It needs cleaning. I should be able to get one up in a bit.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #14) > Created attachment 8466497 [details] [diff] [review] > Use nsExpiration to expire back buffers rebased for v2.0 The new patch should be good.