v0.7.0: startup speed — the 21k-track library stops freezing
Three reported symptoms, one shape: every operation was whole-library, whole-file, on the Qt main thread. Measured on the real library (21,482 tracks, 506 playlists, 15 MB library.json). The track table sorted through a QSortFilterProxyModel, which asks data() for a value on every comparison — 580k Python round trips, 8.5 s per table load, paid again on every reload. TrackTableModel now keeps _tracks in canonical (playlist) order plus an _order index list and sorts a key list computed once per track. Nothing used the proxy's filtering. Sorting by # is the identity order, so "source row" still means "playlist position" for drag-reorder. 8.5 s -> 0.13 s; MainWindow() 18.1 s -> 0.79 s. Smart rules compile to closures once per evaluate() instead of being re-dispatched per track — the operator lookup, casefolding the query and parsing the rule's own date constants all leave the 21k-iteration loop (_match_date was re-parsing its own constant 21,482 times per rule). Verified identical membership against the old evaluator on all 20 real smart playlists. 2.54 s -> 0.72 s, and it now runs after the first paint. Also: _reconcile_track skips two to_dict() round trips per unchanged track (MERGEABLE_FIELDS in the resolver is the single source of truth for what a merge touches); no git subprocess on the startup path (Updater.enabled is probed lazily on the background thread, version-button tooltip deferred); and .resolved/ is pruned to the newest 10 snapshots — it had reached 1.7 GB across 73 snapshots inside the Syncthing share. Time to interactive window ~15 s -> 3.2 s. The mid-session freeze when Syncthing delivers a change — a full reload + recompute + table rebuild with the window already on screen, which is what GNOME was offering to force-quit — ~16 s -> 3.8 s. The merge rework itself (playlist ordering, per-machine play journal, quieting the dialog) is queued in TASKS.md as Round 36. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,8 +24,7 @@ def _manager(tmp_path, n=4):
|
||||
|
||||
def _select_source_row(table, source_row):
|
||||
"""Select the visual row backing a given source-model row (sort-proof)."""
|
||||
proxy_index = table.proxy.mapFromSource(table.model_.index(source_row, 0))
|
||||
table.selectRow(proxy_index.row())
|
||||
table.selectRow(table.model_.display_row(source_row))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
@@ -330,7 +330,7 @@ class TestRatingClicks:
|
||||
emitted = []
|
||||
view.rating_edited.connect(lambda tid, r: emitted.append((tid, r)))
|
||||
col = view.model_.fields.index("rating")
|
||||
rect = view.visualRect(view.proxy.index(0, col))
|
||||
rect = view.visualRect(view.model_.index(0, col))
|
||||
slot_w = rating_slot_width(QFontMetrics(view.font()))
|
||||
# Click the 4th slot → 80.
|
||||
x = rect.left() + RATING_LEFT_PAD + int(slot_w * 3.5)
|
||||
@@ -356,10 +356,10 @@ class TestRatingClicks:
|
||||
track = Track(track_id=1, name="A", rating=40)
|
||||
view = self._view(qapp, [track])
|
||||
col = view.model_.fields.index("rating")
|
||||
index = view.proxy.index(0, col)
|
||||
index = view.model_.index(0, col)
|
||||
view._set_rating_hover(index)
|
||||
assert view.is_rating_hovered(index)
|
||||
assert not view.is_rating_hovered(view.proxy.index(0, 0))
|
||||
assert not view.is_rating_hovered(view.model_.index(0, 0))
|
||||
view._set_rating_hover(None)
|
||||
assert not view.is_rating_hovered(index)
|
||||
|
||||
|
||||
@@ -134,9 +134,12 @@ def button(qapp):
|
||||
return VersionButton(fake), fake
|
||||
|
||||
|
||||
def test_button_shows_version_and_hash(button):
|
||||
def test_button_shows_version_and_hash(qapp, button):
|
||||
btn, fake = button
|
||||
assert btn.text() == f"v{__version__}"
|
||||
# The commit hash is filled in just after construction (short_hash() shells
|
||||
# out to git, so it stays off the startup path).
|
||||
qapp.processEvents()
|
||||
assert "abc1234" in btn.toolTip()
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,231 @@
|
||||
"""Round 35: startup speed.
|
||||
|
||||
The track table used to sort through a QSortFilterProxyModel, which called
|
||||
data() once per comparison — 8.5 s to show a 21k-track library. It now sorts
|
||||
itself over a precomputed key list. Smart-playlist rules are compiled to
|
||||
closures once per evaluate() instead of being re-dispatched per track, and
|
||||
.resolved snapshots are pruned instead of growing without bound.
|
||||
"""
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from PyQt6.QtCore import Qt
|
||||
|
||||
from lintunes.models import Track
|
||||
from lintunes.gui.track_table import TrackTableView, INDEX_FIELD
|
||||
|
||||
|
||||
def _tracks(rows):
|
||||
return [Track(track_id=tid, name=name, artist=artist, play_count=plays)
|
||||
for tid, name, artist, plays in rows]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def view(qapp):
|
||||
table = TrackTableView()
|
||||
table.model_.set_visible_columns(["name", "artist", "play_count"])
|
||||
return table
|
||||
|
||||
|
||||
# ---- in-model sorting -------------------------------------------------------
|
||||
|
||||
class TestSorting:
|
||||
ROWS = [(1, "Ship", "Zoe", 3),
|
||||
(2, "Black Blood", "adele", 10),
|
||||
(3, "Thunder Peel", "Bob", 10),
|
||||
(4, "Sev Beni", None, 1)]
|
||||
|
||||
def test_sorts_case_insensitively(self, view):
|
||||
view.set_tracks(_tracks(self.ROWS))
|
||||
view.sortByColumn(view.model_.fields.index("artist"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
# None sorts with the empty string, then adele < Bob < Zoe (casefolded:
|
||||
# a plain str sort would have put "Bob" and "Zoe" before "adele").
|
||||
assert view.view_order_track_ids() == [4, 2, 3, 1]
|
||||
|
||||
def test_descending_reverses(self, view):
|
||||
view.set_tracks(_tracks(self.ROWS))
|
||||
view.sortByColumn(view.model_.fields.index("artist"),
|
||||
Qt.SortOrder.DescendingOrder)
|
||||
assert view.view_order_track_ids() == [1, 3, 2, 4]
|
||||
|
||||
def test_sort_is_stable_within_ties(self, view):
|
||||
view.set_tracks(_tracks(self.ROWS))
|
||||
view.sortByColumn(view.model_.fields.index("play_count"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
# 2 and 3 both have 10 plays and keep their given order.
|
||||
assert view.view_order_track_ids() == [4, 1, 2, 3]
|
||||
|
||||
def test_mixed_none_and_numbers_do_not_raise(self, view):
|
||||
view.set_tracks([Track(track_id=1, name="A", year=None),
|
||||
Track(track_id=2, name="B", year=1999)])
|
||||
view.model_.set_visible_columns(["name", "year"])
|
||||
view.sortByColumn(view.model_.fields.index("year"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
assert view.view_order_track_ids() == [1, 2]
|
||||
|
||||
def test_set_tracks_keeps_the_current_sort(self, view):
|
||||
view.set_tracks(_tracks(self.ROWS))
|
||||
view.sortByColumn(view.model_.fields.index("artist"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
view.set_tracks(_tracks(self.ROWS[:2]))
|
||||
assert view.view_order_track_ids() == [2, 1]
|
||||
|
||||
def test_selection_survives_a_re_sort(self, view):
|
||||
view.set_tracks(_tracks(self.ROWS))
|
||||
view.sortByColumn(view.model_.fields.index("artist"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
view.selectRow(0) # track 4
|
||||
assert view.selected_track_ids() == [4]
|
||||
view.sortByColumn(view.model_.fields.index("artist"),
|
||||
Qt.SortOrder.DescendingOrder)
|
||||
assert view.selected_track_ids() == [4]
|
||||
|
||||
|
||||
class TestManualOrder:
|
||||
"""A playlist's canonical order must survive sorting: '#' restores it, and
|
||||
'source rows' stay playlist positions so drag-reorder targets the right
|
||||
tracks."""
|
||||
|
||||
ROWS = [(1, "C", "c", 0), (2, "A", "a", 0), (3, "B", "b", 0)]
|
||||
|
||||
@pytest.fixture
|
||||
def playlist_view(self, qapp):
|
||||
table = TrackTableView(playlist_mode=True)
|
||||
table.model_.set_visible_columns(["name"])
|
||||
table.set_tracks(_tracks(self.ROWS))
|
||||
return table
|
||||
|
||||
def test_index_column_restores_manual_order(self, playlist_view):
|
||||
playlist_view.sortByColumn(playlist_view.model_.fields.index("name"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
assert playlist_view.view_order_track_ids() == [2, 3, 1]
|
||||
playlist_view.sortByColumn(
|
||||
playlist_view.model_.fields.index(INDEX_FIELD),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
assert playlist_view.view_order_track_ids() == [1, 2, 3]
|
||||
assert playlist_view.is_manual_sort()
|
||||
|
||||
def test_selection_maps_to_canonical_rows_while_sorted(self, playlist_view):
|
||||
playlist_view.sortByColumn(playlist_view.model_.fields.index("name"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
playlist_view.selectRow(0) # displays "A" = track 2, playlist row 1
|
||||
assert playlist_view.selected_source_rows() == [1]
|
||||
assert playlist_view.selected_track_ids() == [2]
|
||||
|
||||
def test_index_column_numbers_the_displayed_rows(self, playlist_view):
|
||||
playlist_view.sortByColumn(playlist_view.model_.fields.index("name"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
model = playlist_view.model_
|
||||
col = model.fields.index(INDEX_FIELD)
|
||||
assert [model.data(model.index(row, col)) for row in range(3)] == [1, 2, 3]
|
||||
|
||||
def test_reveal_track_finds_a_sorted_row(self, playlist_view):
|
||||
playlist_view.sortByColumn(playlist_view.model_.fields.index("name"),
|
||||
Qt.SortOrder.AscendingOrder)
|
||||
assert playlist_view.reveal_track(1) # "C", last when sorted
|
||||
assert playlist_view.selected_track_ids() == [1]
|
||||
|
||||
|
||||
# ---- compiled smart rules ---------------------------------------------------
|
||||
|
||||
class TestSmartCompilation:
|
||||
"""The rewrite is only safe if it matches the old evaluator exactly."""
|
||||
|
||||
def _criteria(self, rules, conjunction="all"):
|
||||
from lintunes.smart import SmartCriteria, SmartGroup, SmartRule
|
||||
return SmartCriteria(root=SmartGroup(
|
||||
conjunction=conjunction,
|
||||
children=[SmartRule(**r) for r in rules]))
|
||||
|
||||
def test_string_match_is_case_insensitive(self):
|
||||
from lintunes import smart
|
||||
tracks = [Track(track_id=1, name="A", artist="Björk"),
|
||||
Track(track_id=2, name="B", artist="bjork")]
|
||||
criteria = self._criteria(
|
||||
[dict(field="artist", operator="contains", value="BJÖ")])
|
||||
assert smart.evaluate(criteria, tracks) == [1]
|
||||
|
||||
def test_date_rule_constant_is_parsed_once_not_per_track(self):
|
||||
from lintunes import smart
|
||||
tracks = [Track(track_id=1, name="A", play_date_utc="2026-01-01T00:00:00"),
|
||||
Track(track_id=2, name="B", play_date_utc="2020-01-01T00:00:00"),
|
||||
Track(track_id=3, name="C", play_date_utc=None)]
|
||||
criteria = self._criteria([dict(field="last_played", operator="after",
|
||||
value="2025-01-01T00:00:00")])
|
||||
assert smart.evaluate(criteria, tracks) == [1]
|
||||
# A null date is the distant past: "before" matches it, "after" doesn't.
|
||||
criteria = self._criteria([dict(field="last_played", operator="before",
|
||||
value="2025-01-01T00:00:00")])
|
||||
assert smart.evaluate(criteria, tracks) == [2, 3]
|
||||
|
||||
def test_unparseable_date_constant_matches_nothing(self):
|
||||
from lintunes import smart
|
||||
tracks = [Track(track_id=1, name="A", play_date_utc="2026-01-01T00:00:00")]
|
||||
criteria = self._criteria([dict(field="last_played", operator="after",
|
||||
value="not a date")])
|
||||
assert smart.evaluate(criteria, tracks) == []
|
||||
|
||||
def test_in_last_window(self):
|
||||
from lintunes import smart
|
||||
now = datetime(2026, 8, 19, 12, 0, 0)
|
||||
tracks = [Track(track_id=1, name="A", play_date_utc="2026-08-18T12:00:00"),
|
||||
Track(track_id=2, name="B", play_date_utc="2026-01-01T00:00:00"),
|
||||
Track(track_id=3, name="C", play_date_utc=None)]
|
||||
criteria = self._criteria([dict(field="last_played", operator="in_last",
|
||||
value="7", unit="days")])
|
||||
assert smart.evaluate(criteria, tracks, now=now) == [1]
|
||||
criteria = self._criteria([dict(field="last_played",
|
||||
operator="not_in_last",
|
||||
value="7", unit="days")])
|
||||
assert smart.evaluate(criteria, tracks, now=now) == [2, 3]
|
||||
|
||||
def test_any_conjunction(self):
|
||||
from lintunes import smart
|
||||
tracks = [Track(track_id=1, name="A", play_count=50),
|
||||
Track(track_id=2, name="B", artist="Zoe"),
|
||||
Track(track_id=3, name="C")]
|
||||
criteria = self._criteria(
|
||||
[dict(field="play_count", operator="greater", value="10"),
|
||||
dict(field="artist", operator="is", value="Zoe")],
|
||||
conjunction="any")
|
||||
assert smart.evaluate(criteria, tracks) == [1, 2]
|
||||
|
||||
def test_numeric_range_and_bad_values(self):
|
||||
from lintunes import smart
|
||||
tracks = [Track(track_id=1, name="A", play_count=5),
|
||||
Track(track_id=2, name="B", play_count=50)]
|
||||
criteria = self._criteria([dict(field="play_count",
|
||||
operator="in_range",
|
||||
value="40", value2="60")])
|
||||
assert smart.evaluate(criteria, tracks) == [2]
|
||||
criteria = self._criteria([dict(field="play_count", operator="greater",
|
||||
value="not a number")])
|
||||
assert smart.evaluate(criteria, tracks) == []
|
||||
|
||||
|
||||
# ---- .resolved pruning ------------------------------------------------------
|
||||
|
||||
class TestBackupPruning:
|
||||
def test_keeps_only_the_newest_snapshots(self, tmp_path):
|
||||
from lintunes.storage import conflict_resolver
|
||||
root = tmp_path / ".resolved"
|
||||
for stamp in ("20260101-000000", "20260102-000000", "20260103-000000"):
|
||||
(root / stamp / "original").mkdir(parents=True)
|
||||
(root / stamp / "original" / "library.json").write_text("{}")
|
||||
conflict_resolver._prune_backups(tmp_path, keep=2)
|
||||
assert sorted(d.name for d in root.iterdir()) == ["20260102-000000",
|
||||
"20260103-000000"]
|
||||
|
||||
def test_prune_runs_even_with_no_conflicts(self, tmp_path):
|
||||
from lintunes.storage import conflict_resolver
|
||||
root = tmp_path / ".resolved"
|
||||
for i in range(conflict_resolver.BACKUPS_TO_KEEP + 3):
|
||||
(root / f"2026010{i // 10}-00000{i % 10}").mkdir(parents=True)
|
||||
assert conflict_resolver.resolve_conflicts(tmp_path) == []
|
||||
assert len(list(root.iterdir())) == conflict_resolver.BACKUPS_TO_KEEP
|
||||
|
||||
def test_missing_backup_dir_is_fine(self, tmp_path):
|
||||
from lintunes.storage import conflict_resolver
|
||||
conflict_resolver._prune_backups(tmp_path) # must not raise
|
||||
Reference in New Issue
Block a user