Prefetching a lot more stuff to reduce the number of SQL queries

sublime/adapters/adapter_base.py

@@ -815,7 +815,9 @@ class CachingAdapter(Adapter):
     # Cache-Specific Methods
     # ==================================================================================
     @abc.abstractmethod
-    def get_cached_statuses(self, songs: Sequence[Song]) -> Dict[str, SongCacheStatus]:
+    def get_cached_statuses(
+        self, song_ids: Sequence[str]
+    ) -> Dict[str, SongCacheStatus]:
         """
         Returns the cache statuses for the given list of songs. See the
         :class:`SongCacheStatus` documentation for more details about what each status

sublime/adapters/filesystem/adapter.py

@@ -1,5 +1,4 @@
 import hashlib
-import itertools
 import logging
 import shutil
 import threading
@@ -7,7 +6,7 @@ from datetime import datetime
 from pathlib import Path
 from typing import Any, cast, Dict, Optional, Sequence, Set, Tuple, Union
 
-from peewee import fn
+from peewee import fn, prefetch
 
 from sublime.adapters import api_objects as API
 
@@ -194,7 +193,7 @@ class FilesystemAdapter(CachingAdapter):
     # Data Retrieval Methods
     # ==================================================================================
     def get_cached_statuses(
-        self, songs: Sequence[API.Song]
+        self, song_ids: Sequence[str]
     ) -> Dict[str, SongCacheStatus]:
         def compute_song_cache_status(song: models.Song) -> SongCacheStatus:
             file = song.file
@@ -209,18 +208,18 @@ class FilesystemAdapter(CachingAdapter):
             return SongCacheStatus.NOT_CACHED
 
         try:
-            song_models = (
+            file_models = models.CacheInfo.select().where(
-                songs
+                models.CacheInfo.cache_key == KEYS.SONG_FILE
-                if isinstance(songs[0], models.Song)
-                else models.Song.select().where(
-                    models.Song.id.in_([song.id for song in songs])
-                )
             )
-            return {s.id: compute_song_cache_status(s) for s in song_models}
+            song_models = models.Song.select().where(models.Song.id.in_(song_ids))
+            return {
+                s.id: compute_song_cache_status(s)
+                for s in prefetch(song_models, file_models)
+            }
         except Exception:
             pass
 
-        return {song.id: SongCacheStatus.NOT_CACHED for song in songs}
+        return {song_id: SongCacheStatus.NOT_CACHED for song_id in song_ids}
 
     _playlists = None
 
@@ -623,7 +622,7 @@ class FilesystemAdapter(CachingAdapter):
                 "comment": getattr(api_playlist, "comment", None),
                 "owner": getattr(api_playlist, "owner", None),
                 "public": getattr(api_playlist, "public", None),
-                "songs": [
+                "_songs": [
                     self._do_ingest_new_data(KEYS.SONG, s.id, s)
                     for s in api_playlist.songs
                 ],

sublime/adapters/filesystem/models.py

@@ -6,6 +6,7 @@ from peewee import (
     ForeignKeyField,
     IntegerField,
     Model,
+    prefetch,
     Query,
     SqliteDatabase,
     TextField,
@@ -184,7 +185,13 @@ class Playlist(BaseModel):
     changed = TzDateTimeField(null=True)
     public = BooleanField(null=True)
 
-    songs = SortedManyToManyField(Song, backref="playlists")
+    _songs = SortedManyToManyField(Song, backref="playlists")
+
+    @property
+    def songs(self) -> List[Song]:
+        albums = Album.select()
+        artists = Album.select()
+        return prefetch(self._songs, albums, artists)
 
     _cover_art = ForeignKeyField(CacheInfo, null=True)
 
@@ -230,7 +237,7 @@ ALL_TABLES = (
     Genre,
     IgnoredArticle,
     Playlist,
-    Playlist.songs.get_through_model(),
+    Playlist._songs.get_through_model(),
     SimilarArtist,
     Song,
     Version,

sublime/adapters/manager.py

@@ -1196,17 +1196,17 @@ class AdapterManager:
     # Cache Status Methods
     # ==================================================================================
     @staticmethod
-    def get_cached_statuses(songs: Sequence[Song]) -> Sequence[SongCacheStatus]:
+    def get_cached_statuses(song_ids: Sequence[str]) -> Sequence[SongCacheStatus]:
         assert AdapterManager._instance
         if not AdapterManager._instance.caching_adapter:
-            return list(itertools.repeat(SongCacheStatus.NOT_CACHED, len(songs)))
+            return list(itertools.repeat(SongCacheStatus.NOT_CACHED, len(song_ids)))
 
         cached_statuses = AdapterManager._instance.caching_adapter.get_cached_statuses(
-            songs
+            song_ids
         )
         return [
             SongCacheStatus.DOWNLOADING
-            if song.id in AdapterManager.current_download_ids
+            if song_id in AdapterManager.current_download_ids
-            else cached_statuses[song.id]
+            else cached_statuses[song_id]
-            for song in songs
+            for song_id in song_ids
         ]

sublime/adapters/subsonic/api_objects.py

@@ -204,26 +204,11 @@ class Song(SublimeAPI.Song, DataClassJsonMixin):
 @dataclass_json(letter_case=LetterCase.CAMEL)
 @dataclass
 class Playlist(SublimeAPI.Playlist):
-    id: str
-    name: str
-    song_count: Optional[int] = None
-    duration: Optional[timedelta] = None
-    created: Optional[datetime] = None
-    changed: Optional[datetime] = None
-    comment: Optional[str] = None
-    owner: Optional[str] = None
-    public: Optional[bool] = None
-    cover_art: Optional[str] = None
-
-
-@dataclass_json(letter_case=LetterCase.CAMEL)
-@dataclass
-class PlaylistWithSongs(SublimeAPI.Playlist):
     id: str
     name: str
     songs: List[Song] = field(default_factory=list, metadata=config(field_name="entry"))
-    song_count: int = field(default=0)
+    song_count: Optional[int] = field(default=None)
-    duration: timedelta = field(default=timedelta())
+    duration: Optional[timedelta] = field(default=None)
     created: Optional[datetime] = None
     changed: Optional[datetime] = None
     comment: Optional[str] = None
@@ -232,12 +217,17 @@ class PlaylistWithSongs(SublimeAPI.Playlist):
     cover_art: Optional[str] = None
 
     def __post_init__(self):
-        self.song_count = self.song_count or len(self.songs)
+        if self.songs is None:
-        self.duration = self.duration or timedelta(
+            return
-            seconds=sum(
+        if self.song_count is None:
-                s.duration.total_seconds() if s.duration else 0 for s in self.songs
+            self.song_count = len(self.songs)
+
+        if self.duration is None:
+            self.duration = timedelta(
+                seconds=sum(
+                    s.duration.total_seconds() if s.duration else 0 for s in self.songs
+                )
             )
-        )
 
 
 @dataclass_json(letter_case=LetterCase.CAMEL)
@@ -336,7 +326,7 @@ class Response(DataClassJsonMixin):
 
     indexes: Optional[Indexes] = None
 
-    playlist: Optional[PlaylistWithSongs] = None
+    playlist: Optional[Playlist] = None
     playlists: Optional[Playlists] = None
 
     play_queue: Optional[PlayQueue] = field(

sublime/app.py

@@ -206,11 +206,13 @@ class SublimeMusicApp(Gtk.Application):
                 self.app_config.state.playing = event.playing
                 if self.dbus_manager:
                     self.dbus_manager.property_diff()
+                self.update_window()
             elif event.type == PlayerEvent.Type.VOLUME_CHANGE:
                 assert event.volume
                 self.app_config.state.volume = event.volume
                 if self.dbus_manager:
                     self.dbus_manager.property_diff()
+                self.update_window()
             elif event.type == PlayerEvent.Type.STREAM_CACHE_PROGRESS_CHANGE:
                 if (
                     self.loading_state
@@ -229,8 +231,6 @@ class SublimeMusicApp(Gtk.Application):
                     self.app_config.state.song_stream_cache_progress,
                 )
 
-            self.update_window()
-
         self.mpv_player = MPVPlayer(
             time_observer, on_track_end, on_player_event, self.app_config,
         )
@@ -1151,7 +1151,6 @@ class SublimeMusicApp(Gtk.Application):
         )
 
     def save_play_queue(self):
-        # TODO let this be delayed as well
         if len(self.app_config.state.play_queue) == 0:
             return
 

sublime/ui/browse.py

@@ -250,7 +250,6 @@ class MusicDirectoryList(Gtk.Box):
         )
 
     _current_child_ids: List[str] = []
-    songs: List[API.Song] = []
 
     @util.async_callback(
         AdapterManager.get_directory,
@@ -275,7 +274,7 @@ class MusicDirectoryList(Gtk.Box):
         # The entire algorithm ends up being O(2n), but the first loop is very tight,
         # and the expensive parts of the second loop are avoided if the IDs haven't
         # changed.
-        children_ids, children = [], []
+        children_ids, children, song_ids = [], [], []
         selected_dir_idx = None
         for i, c in enumerate(directory.children):
             if i >= len(self._current_child_ids) or c.id != self._current_child_ids[i]:
@@ -287,18 +286,21 @@ class MusicDirectoryList(Gtk.Box):
             children_ids.append(c.id)
             children.append(c)
 
+            if not hasattr(c, "children"):
+                song_ids.append(c.id)
+
         if force:
             new_directories_store = []
             self._current_child_ids = children_ids
 
-            self.songs = []
+            songs = []
             for el in children:
                 if hasattr(el, "children"):
                     new_directories_store.append(
                         MusicDirectoryList.DrilldownElement(cast(API.Directory, el))
                     )
                 else:
-                    self.songs.append(cast(API.Song, el))
+                    songs.append(cast(API.Song, el))
 
             util.diff_model_store(
                 self.drilldown_directories_store, new_directories_store
@@ -312,14 +314,14 @@ class MusicDirectoryList(Gtk.Box):
                     song.id,
                 ]
                 for status_icon, song in zip(
-                    util.get_cached_status_icons(self.songs), self.songs
+                    util.get_cached_status_icons(song_ids), songs
                 )
             ]
         else:
             new_songs_store = [
                 [status_icon] + song_model[1:]
                 for status_icon, song_model in zip(
-                    util.get_cached_status_icons(self.songs), self.directory_song_store
+                    util.get_cached_status_icons(song_ids), self.directory_song_store
                 )
             ]
 

sublime/ui/common/album_with_songs.py

@@ -266,6 +266,7 @@ class AlbumWithSongs(Gtk.Box):
         force: bool = False,
         order_token: int = None,
     ):
+        song_ids = [s.id for s in album.songs or []]
         new_store = [
             [
                 cached_status,
@@ -274,7 +275,7 @@ class AlbumWithSongs(Gtk.Box):
                 song.id,
             ]
             for cached_status, song in zip(
-                util.get_cached_status_icons(list(album.songs or [])), album.songs or []
+                util.get_cached_status_icons(song_ids), album.songs or []
             )
         ]
 

sublime/ui/player_controls.py

@@ -206,7 +206,6 @@ class PlayerControls(Gtk.ActionBar):
         def make_idle_index_capturing_function(
             idx: int, order_tok: int, fn: Callable[[int, int, Any], None],
         ) -> Callable[[Result], None]:
-            # TODO use partial here?
             return lambda f: GLib.idle_add(fn, idx, order_tok, f.result())
 
         def on_cover_art_future_done(

sublime/ui/playlists.py

@@ -436,7 +436,6 @@ class PlaylistDetailPanel(Gtk.Overlay):
             )
 
     _current_song_ids: List[str] = []
-    songs: List[API.Song] = []
 
     @util.async_callback(
         AdapterManager.get_playlist_details,
@@ -494,7 +493,6 @@ class PlaylistDetailPanel(Gtk.Overlay):
 
         if force:
             self._current_song_ids = song_ids
-            self.songs = [cast(API.Song, s) for s in songs]
 
             new_songs_store = [
                 [
@@ -506,14 +504,15 @@ class PlaylistDetailPanel(Gtk.Overlay):
                     song.id,
                 ]
                 for status_icon, song in zip(
-                    util.get_cached_status_icons(self.songs), self.songs
+                    util.get_cached_status_icons(song_ids),
+                    [cast(API.Song, s) for s in songs],
                 )
             ]
         else:
             new_songs_store = [
                 [status_icon] + song_model[1:]
                 for status_icon, song_model in zip(
-                    util.get_cached_status_icons(self.songs), self.playlist_song_store
+                    util.get_cached_status_icons(song_ids), self.playlist_song_store
                 )
             ]
 

sublime/ui/util.py

@@ -67,7 +67,7 @@ def format_sequence_duration(duration: Optional[timedelta]) -> str:
 
     >>> format_sequence_duration(timedelta(seconds=90))
     '1 minute, 30 seconds'
-    >>> format_sequence_duration(seconds=(60 * 60 + 120))
+    >>> format_sequence_duration(timedelta(seconds=(60 * 60 + 120)))
     '1 hour, 2 minutes'
     >>> format_sequence_duration(None)
     '0 seconds'
@@ -116,7 +116,7 @@ def dot_join(*items: Any) -> str:
     return "  •  ".join(map(str, filter(lambda x: x is not None, items)))
 
 
-def get_cached_status_icons(songs: List[Song]) -> List[str]:
+def get_cached_status_icons(song_ids: List[str]) -> List[str]:
     cache_icon = {
         SongCacheStatus.CACHED: "folder-download-symbolic",
         SongCacheStatus.PERMANENTLY_CACHED: "view-pin-symbolic",
@@ -124,7 +124,7 @@ def get_cached_status_icons(songs: List[Song]) -> List[str]:
     }
     return [
         cache_icon.get(cache_status, "")
-        for cache_status in AdapterManager.get_cached_statuses(songs)
+        for cache_status in AdapterManager.get_cached_statuses(song_ids)
     ]
 
 
@@ -173,7 +173,6 @@ def diff_model_store(store_to_edit: Any, new_store: Iterable[Any]):
     The diff here is that if there are any differences, then we refresh the
     entire list. This is because it is too hard to do editing.
     """
-    # TODO: figure out if there's a way to do editing.
     old_store = store_to_edit[:]
 
     diff = DeepDiff(old_store, new_store)
@@ -224,7 +223,7 @@ def show_song_popover(
     song_details = [
         AdapterManager.get_song_details(song_id).result() for song_id in song_ids
     ]
-    song_cache_statuses = AdapterManager.get_cached_statuses(song_details)
+    song_cache_statuses = AdapterManager.get_cached_statuses(song_ids)
     for song, status in zip(song_details, song_cache_statuses):
         # TODO lazy load these
         albums.add(album.id if (album := song.album) else None)

tests/adapter_tests/filesystem_adapter_tests.py

@@ -194,7 +194,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "1",
-        SubsonicAPI.PlaylistWithSongs("1", "test1", songs=MOCK_SUBSONIC_SONGS[:2]),
+        SubsonicAPI.Playlist("1", "test1", songs=MOCK_SUBSONIC_SONGS[:2]),
     )
 
     playlist = cache_adapter.get_playlist_details("1")
@@ -208,7 +208,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "1",
-        SubsonicAPI.PlaylistWithSongs("1", "foo", songs=MOCK_SUBSONIC_SONGS),
+        SubsonicAPI.Playlist("1", "foo", songs=MOCK_SUBSONIC_SONGS),
     )
 
     playlist = cache_adapter.get_playlist_details("1")
@@ -254,13 +254,13 @@ def test_caching_get_playlist_then_details(cache_adapter: FilesystemAdapter):
 
     # Simulate getting playlist details for id=1, then id=2
     cache_adapter.ingest_new_data(
-        KEYS.PLAYLIST_DETAILS, "1", SubsonicAPI.PlaylistWithSongs("1", "test1"),
+        KEYS.PLAYLIST_DETAILS, "1", SubsonicAPI.Playlist("1", "test1"),
     )
 
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "2",
-        SubsonicAPI.PlaylistWithSongs("2", "test2", songs=MOCK_SUBSONIC_SONGS),
+        SubsonicAPI.Playlist("2", "test2", songs=MOCK_SUBSONIC_SONGS),
     )
 
     # Going back and getting playlist details for the first one should not
@@ -295,7 +295,7 @@ def test_invalidate_playlist(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "2",
-        SubsonicAPI.PlaylistWithSongs("2", "test2", cover_art="pl_2", songs=[]),
+        SubsonicAPI.Playlist("2", "test2", cover_art="pl_2", songs=[]),
     )
     cache_adapter.ingest_new_data(
         KEYS.COVER_ART_FILE, "pl_2", MOCK_ALBUM_ART2,
@@ -381,41 +381,35 @@ def test_malformed_song_path(cache_adapter: FilesystemAdapter):
 
 def test_get_cached_statuses(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(KEYS.SONG, "1", MOCK_SUBSONIC_SONGS[1])
-    assert cache_adapter.get_cached_statuses([cache_adapter.get_song_details("1")]) == [
+    assert cache_adapter.get_cached_statuses(["1"]) == {"1": SongCacheStatus.NOT_CACHED}
-        SongCacheStatus.NOT_CACHED
-    ]
 
     cache_adapter.ingest_new_data(KEYS.SONG_FILE, "1", (None, MOCK_SONG_FILE))
-    assert cache_adapter.get_cached_statuses([cache_adapter.get_song_details("1")]) == [
+    assert cache_adapter.get_cached_statuses(["1"]) == {"1": SongCacheStatus.CACHED}
-        SongCacheStatus.CACHED
-    ]
 
     cache_adapter.ingest_new_data(KEYS.SONG_FILE_PERMANENT, "1", None)
-    assert cache_adapter.get_cached_statuses([cache_adapter.get_song_details("1")]) == [
+    assert cache_adapter.get_cached_statuses(["1"]) == {
-        SongCacheStatus.PERMANENTLY_CACHED
+        "1": SongCacheStatus.PERMANENTLY_CACHED
-    ]
+    }
 
     cache_adapter.invalidate_data(KEYS.SONG_FILE, "1")
-    assert cache_adapter.get_cached_statuses([cache_adapter.get_song_details("1")]) == [
+    assert cache_adapter.get_cached_statuses(["1"]) == {
-        SongCacheStatus.CACHED_STALE
+        "1": SongCacheStatus.CACHED_STALE
-    ]
+    }
 
     cache_adapter.delete_data(KEYS.SONG_FILE, "1")
-    assert cache_adapter.get_cached_statuses([cache_adapter.get_song_details("1")]) == [
+    assert cache_adapter.get_cached_statuses(["1"]) == {"1": SongCacheStatus.NOT_CACHED}
-        SongCacheStatus.NOT_CACHED
-    ]
 
 
 def test_delete_playlists(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "1",
-        SubsonicAPI.PlaylistWithSongs("1", "test1", cover_art="pl_1", songs=[]),
+        SubsonicAPI.Playlist("1", "test1", cover_art="pl_1", songs=[]),
     )
     cache_adapter.ingest_new_data(
         KEYS.PLAYLIST_DETAILS,
         "2",
-        SubsonicAPI.PlaylistWithSongs("2", "test1", cover_art="pl_2", songs=[]),
+        SubsonicAPI.Playlist("2", "test1", cover_art="pl_2", songs=[]),
     )
     cache_adapter.ingest_new_data(
         KEYS.COVER_ART_FILE, "pl_1", MOCK_ALBUM_ART,