From 2df05144688f847103ee2422dec0c744056b235c Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Tue, 11 Jul 2023 16:38:50 -0700 Subject: [PATCH] add cleanup task to widget pool --- ui/browsing/router.go | 22 +++++++-------- ui/util/widgetpool.go | 58 +++++++++++++++++++++++++++++++------- ui/util/widgetpool_test.go | 24 ++++++++++++++++ 3 files changed, 83 insertions(+), 21 deletions(-) create mode 100644 ui/util/widgetpool_test.go diff --git a/ui/browsing/router.go b/ui/browsing/router.go index 67dfa99..631d6f8 100644 --- a/ui/browsing/router.go +++ b/ui/browsing/router.go @@ -15,7 +15,7 @@ type Router struct { App *backend.App Controller *controller.Controller Nav NavigationHandler - widgetPool util.WidgetPool + widgetPool *util.WidgetPool } func NewRouter(app *backend.App, controller *controller.Controller, nav NavigationHandler) Router { @@ -31,27 +31,27 @@ func NewRouter(app *backend.App, controller *controller.Controller, nav Navigati func (r Router) CreatePage(rte controller.Route) Page { switch rte.Page { case controller.Album: - return NewAlbumPage(rte.Arg, &r.App.Config.AlbumPage, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller) + return NewAlbumPage(rte.Arg, &r.App.Config.AlbumPage, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller) case controller.Albums: - return NewAlbumsPage(&r.App.Config.AlbumsPage, &r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) + return NewAlbumsPage(&r.App.Config.AlbumsPage, r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) case controller.Artist: - return NewArtistPage(rte.Arg, &r.App.Config.ArtistPage, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller) + return NewArtistPage(rte.Arg, &r.App.Config.ArtistPage, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller) case controller.Artists: - return NewArtistsPage(r.Controller, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) + return NewArtistsPage(r.Controller, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) case controller.Favorites: - return NewFavoritesPage(&r.App.Config.FavoritesPage, &r.widgetPool, r.Controller, r.App.ServerManager.Server, r.App.PlaybackManager, r.App.ImageManager) + return NewFavoritesPage(&r.App.Config.FavoritesPage, r.widgetPool, r.Controller, r.App.ServerManager.Server, r.App.PlaybackManager, r.App.ImageManager) case controller.Genre: - return NewGenrePage(rte.Arg, &r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) + return NewGenrePage(rte.Arg, r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager) case controller.Genres: return NewGenresPage(r.Controller, r.App.ServerManager.Server) case controller.NowPlaying: - return NewNowPlayingPage(rte.Arg, r.Controller, &r.widgetPool, &r.App.Config.NowPlayingPage, r.App.PlaybackManager, r.App.Player) + return NewNowPlayingPage(rte.Arg, r.Controller, r.widgetPool, &r.App.Config.NowPlayingPage, r.App.PlaybackManager, r.App.Player) case controller.Playlist: - return NewPlaylistPage(rte.Arg, &r.App.Config.PlaylistPage, &r.widgetPool, r.Controller, r.App.ServerManager, r.App.PlaybackManager, r.App.ImageManager) + return NewPlaylistPage(rte.Arg, &r.App.Config.PlaylistPage, r.widgetPool, r.Controller, r.App.ServerManager, r.App.PlaybackManager, r.App.ImageManager) case controller.Playlists: - return NewPlaylistsPage(r.Controller, &r.widgetPool, &r.App.Config.PlaylistsPage, r.App.ServerManager.Server) + return NewPlaylistsPage(r.Controller, r.widgetPool, &r.App.Config.PlaylistsPage, r.App.ServerManager.Server) case controller.Tracks: - return NewTracksPage(r.Controller, &r.App.Config.TracksPage, &r.widgetPool, r.App.ServerManager.Server) + return NewTracksPage(r.Controller, &r.App.Config.TracksPage, r.widgetPool, r.App.ServerManager.Server) } return nil } diff --git a/ui/util/widgetpool.go b/ui/util/widgetpool.go index ea73306..5a81d12 100644 --- a/ui/util/widgetpool.go +++ b/ui/util/widgetpool.go @@ -1,6 +1,7 @@ package util import ( + "sync" "time" "fyne.io/fyne/v2" @@ -19,11 +20,16 @@ const ( numWidgetTypes ) +const ( + multipleItemsExpiry = 2 * time.Minute + singleItemExpiry = 5 * time.Minute +) + // A pool to share commonly-used widgets across pages to reduce // creation of new widgets and memory allocations. -// It is not thread-safe, which is fine for its current use. type WidgetPool struct { - pool [][]pooledWidget + mut sync.Mutex + pools [][]pooledWidget } type pooledWidget struct { @@ -31,21 +37,30 @@ type pooledWidget struct { releasedAt int64 // unixMillis } -func NewWidgetPool() WidgetPool { - return WidgetPool{ - pool: make([][]pooledWidget, numWidgetTypes), +func NewWidgetPool() *WidgetPool { + p := &WidgetPool{ + pools: make([][]pooledWidget, numWidgetTypes), } + go func() { + t := time.NewTicker(2 * time.Minute) + for range t.C { + p.cleanUpExpiredItems() + } + }() + return p } // Obtain obtains a widget of the given type from the pool, if one exists. // Returns nil if there is no available widget. func (w *WidgetPool) Obtain(typ WidgetType) fyne.CanvasObject { + w.mut.Lock() + defer w.mut.Unlock() var widget fyne.CanvasObject - if l := len(w.pool[typ]); l > 0 { + if l := len(w.pools[typ]); l > 0 { i := l - 1 - widget = w.pool[typ][i].widget - w.pool[typ][i].widget = nil - w.pool[typ] = w.pool[typ][:i] + widget = w.pools[typ][i].widget + w.pools[typ][i].widget = nil + w.pools[typ] = w.pools[typ][:i] } return widget } @@ -54,8 +69,31 @@ func (w *WidgetPool) Obtain(typ WidgetType) fyne.CanvasObject { // The widget must not be modified by the releaser after release, // since it may be Obtained for a new use at any time. func (w *WidgetPool) Release(typ WidgetType, wid fyne.CanvasObject) { - w.pool[typ] = append(w.pool[typ], pooledWidget{ + w.mut.Lock() + defer w.mut.Unlock() + w.pools[typ] = append(w.pools[typ], pooledWidget{ widget: wid, releasedAt: time.Now().UnixMilli(), }) } + +func (w *WidgetPool) cleanUpExpiredItems() { + w.mut.Lock() + defer w.mut.Unlock() + for widTyp, pool := range w.pools { + newP := make([]pooledWidget, 0, len(pool)) + l := len(pool) + for _, wid := range pool { + timeSinceRelease := time.Since(time.UnixMilli(wid.releasedAt)) + if l > 1 && timeSinceRelease > multipleItemsExpiry { + l-- // let expire if >1 item in pool and released long enough ago + } else if l == 1 && timeSinceRelease > singleItemExpiry { + l-- // let expire if last item in pool and released long enough ago + } else { + newP = append(newP, wid) // not expired + } + } + // re-assign non-expired items back to this widget type pool + w.pools[widTyp] = newP + } +} diff --git a/ui/util/widgetpool_test.go b/ui/util/widgetpool_test.go new file mode 100644 index 0000000..79ea851 --- /dev/null +++ b/ui/util/widgetpool_test.go @@ -0,0 +1,24 @@ +package util + +import ( + "testing" + "time" +) + +func Test_WidgetPool_CleanupExpiredItems(t *testing.T) { + now := time.Now() + threeMinAgo := now.Add(-3 * time.Minute) + w := &WidgetPool{ + pools: [][]pooledWidget{ + { + {releasedAt: threeMinAgo.UnixMilli()}, + {releasedAt: threeMinAgo.UnixMilli()}, + {releasedAt: threeMinAgo.UnixMilli()}, + }, + }, + } + w.cleanUpExpiredItems() + if l := len(w.pools[0]); l != 1 { + t.Errorf("Expected one widget in pool after cleanup, got %d", l) + } +}