From a4c29c796c7e4f87a1f511cd296e846dcd6fafdf Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:49:11 -0600 Subject: [PATCH] perf(qt): build the tab that is showing, not all of them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entering Discover froze the UI thread for 1.52s — not slow, frozen: zero pixels changing anywhere in the window — where the Slint build painted the same page, artwork included, in a single frame. Measured frame by frame from a screen recording, with the click located at the moment the nav flyout disappears. `visible: false` hides an item; it does not stop QML from building it. HomeView declared its four Discover tabs as sibling Columns gated only on `visible:`, so every mount instantiated all four and 19 of its 28 section rails belonged to tabs nobody was looking at. Gating each body on a Loader's `active:` cut a Home mount from 426-512ms to 182-190ms, and the tab block itself from 364-444ms to 134-140ms. Synchronous, deliberately: incubation is time-sliced, so `asynchronous` spreads the same work over more frames instead of removing it. Three more of the same class, found by auditing for it: - SlimGrid's delegate Loader had no `active:`, so it built a full card for the items past `total` and then hid them. - LocalLibraryView mounted all four tab bodies, the Tracks one being the 16K-row view. - AlbumInfoModal built a credits row per track of the album even when the Description tab was showing. ContentRouter's header claimed the fix was to keep views alive across navigation. It is not, and the references say so: Slint destroys its view tree with `if` (AppShell.slint:426) and Tauri removes it from the DOM (+page.svelte:6286), and Tauri even refetches from the network on mount while still transitioning instantly. Neither gets its speed from caching the view — both keep the mount proportional to one viewport. The rule is the SIZE of a mount, not its lifetime. Paragraph rewritten to say that. cc --- crates/qbz-qt/qml/shell/AlbumInfoModal.qml | 31 ++- crates/qbz-qt/qml/shell/ContentRouter.qml | 76 +++++++- crates/qbz-qt/qml/views/HomeView.qml | 192 ++++++++++++++----- crates/qbz-qt/qml/views/LocalLibraryView.qml | 36 ++-- 4 files changed, 254 insertions(+), 81 deletions(-) diff --git a/crates/qbz-qt/qml/shell/AlbumInfoModal.qml b/crates/qbz-qt/qml/shell/AlbumInfoModal.qml index d821b0d28..5ad6c8041 100644 --- a/crates/qbz-qt/qml/shell/AlbumInfoModal.qml +++ b/crates/qbz-qt/qml/shell/AlbumInfoModal.qml @@ -476,17 +476,28 @@ Popup { width: flick.width - 18 spacing: 0 - Column { - visible: root.activeTab === "credits" + // Loader-gated, not `visible:`-gated: a + // hidden Column is still BUILT, so opening + // this modal on the Description tab was + // constructing one CreditTrackRow per + // track of the album regardless — the + // whole point of a credits tab being that + // a box set has a lot of them. + Loader { width: parent.width - spacing: 0 - Repeater { - model: root.doc.tracks || [] - delegate: CreditTrackRow { - required property var modelData - required property int index - track: modelData - isLast: index === (root.doc.tracks || []).length - 1 + active: root.activeTab === "credits" + visible: active + sourceComponent: Column { + width: parent.width + spacing: 0 + Repeater { + model: root.doc.tracks || [] + delegate: CreditTrackRow { + required property var modelData + required property int index + track: modelData + isLast: index === (root.doc.tracks || []).length - 1 + } } } } diff --git a/crates/qbz-qt/qml/shell/ContentRouter.qml b/crates/qbz-qt/qml/shell/ContentRouter.qml index 631c73e5b..186e87d26 100644 --- a/crates/qbz-qt/qml/shell/ContentRouter.qml +++ b/crates/qbz-qt/qml/shell/ContentRouter.qml @@ -66,14 +66,62 @@ Item { // On a top-level tab the user expects to be instant, that trade is simply // the wrong one. // - // The real fix is NOT here: it is to stop rebuilding these views at all - // (keep the top-level tabs alive across navigation) and to make the heavy - // views build incrementally. Until one of those exists, synchronous is the - // honest behaviour — the same one that shipped before 2026-08-13. + // WHAT THE REAL FIX TURNED OUT TO BE — and this paragraph used to say the + // opposite, so read it before acting on a memory of it. It claimed the + // answer was to "keep the top-level tabs alive across navigation". That is + // wrong, and it was wrong on the evidence available at the time: BOTH + // references destroy and re-create the view exactly like this Loader does. + // Slint gates every view on `if NavState.view == ...` (AppShell.slint:426), + // which destroys the element tree; Tauri used `{#if activeView === 'home'}` + // (+page.svelte:6286), which removes it from the DOM — and Tauri even + // REFETCHES from the network on mount and still transitions instantly. + // Neither gets its speed from caching the view. + // + // What they both do is keep the MOUNT proportional to one viewport, and + // that is where this port had diverged. Measured 2026-08-17 by driving the + // real app (`QBZ_QT_NAV_BENCH`, the probe below, and a frame-by-frame read + // of a screen recording): a click on Discover froze the UI thread for + // 1.52 s — not slow, FROZEN, zero pixels changing anywhere in the window — + // where the Slint build painted the same page, artwork included, in a + // single frame. Inside that time HomeView was building all FOUR of its + // Discover tabs, because `visible: false` hides an item and does not stop + // QML from instantiating it, so 19 of 28 section rails belonged to tabs + // nobody was looking at. Slint runs ONE repeater whose model is chosen by + // a ternary (HomeView.slint:321) and mounts the rest behind `if`. + // + // So the rule is about the SIZE of a mount, not its lifetime: a view may + // be rebuilt freely as long as it only builds what is on screen. That rule + // is now enforced statically — qml_eager_tab_audit.py runs in qt-run.sh + // and fails the build on a heavy per-tab body gated only by `visible:`. + // Synchronous stays, and is correct. + // Route-change stopwatch. OFF by default (a LoggingCategory at Warning + // emits nothing at Info), so the only cost on a normal run is one + // Date.now() per route change. Turn it on with: + // + // QT_LOGGING_RULES="qbz.nav.timing.info=true" ./target/release/qbz + // + // It answers the question no static gate can: of the wall-clock between + // the click and a usable page, how much is QML instantiation (`built`) + // versus everything the event loop still owes afterwards (`to-idle`). + LoggingCategory { + id: navTiming + name: "qbz.nav.timing" + defaultLogLevel: LoggingCategory.Warning + } + property double _routeT0: 0 + // Stamped from INSIDE the source binding rather than from a + // currentViewChanged handler: binding re-evaluation and signal handlers + // have no guaranteed order, and this way the stamp is by construction the + // last thing before the Loader instantiates. + function _stampRoute(path) { + root._routeT0 = Date.now() + return path + } + Loader { id: viewLoader anchors.fill: parent - source: QbzShell.currentView === "home" + source: root._stampRoute(QbzShell.currentView === "home" ? (root.kiosk ? "../kiosk/KioskDiscover.qml" : "../views/HomeView.qml") : QbzShell.currentView === "library" ? (root.kiosk ? "../kiosk/KioskLibrary.qml" : "../views/LibraryView.qml") @@ -171,7 +219,23 @@ Item { // its transport is the persistent bar — so outside kiosk this id // falls through to "" and the pane is blank, which is the desktop // behaviour before this router existed. - : root.kiosk && QbzShell.currentView === "nowplaying" ? "../kiosk/KioskNowPlaying.qml" : "" + : root.kiosk && QbzShell.currentView === "nowplaying" ? "../kiosk/KioskNowPlaying.qml" : "") + + onLoaded: { + var built = Date.now() - root._routeT0 + console.info(navTiming, "[navtiming] " + QbzShell.currentView + + " built=" + built + "ms") + // One more turn of the event loop: everything the mount deferred + // (layout, the first delegate refills, the pending property + // updates) has run by the time this fires, so the delta is the + // honest "how long until the page stops owing work" number. + var t0 = root._routeT0 + var view = QbzShell.currentView + Qt.callLater(function () { + console.info(navTiming, "[navtiming] " + view + + " to-idle=" + (Date.now() - t0) + "ms") + }) + } } // MyQbzGridView serves BOTH the "mixtapes" and "collections" routes diff --git a/crates/qbz-qt/qml/views/HomeView.qml b/crates/qbz-qt/qml/views/HomeView.qml index 29a1b0559..ff2f9af8f 100644 --- a/crates/qbz-qt/qml/views/HomeView.qml +++ b/crates/qbz-qt/qml/views/HomeView.qml @@ -67,6 +67,25 @@ Rectangle { QbzTheme { id: theme } + // ---- mount stopwatch (same category as ContentRouter's) -------------- + // OFF unless QT_LOGGING_RULES="qbz.nav.timing.info=true". It exists to + // attribute a Home mount: this view declares FOUR Discover tabs and QML + // instantiates all four (`visible: false` hides a Column, it does not stop + // it from being built), so the question "how much of the wait is the three + // tabs nobody asked for" needs per-tab numbers, not a single total. + LoggingCategory { + id: homeTiming + name: "qbz.nav.timing" + defaultLogLevel: LoggingCategory.Warning + } + // A property initialiser runs during creation, so this is stamped near the + // start of the mount rather than at the end of it like Component.onCompleted. + property double _mountT0: Date.now() + function _stampTab(id, count) { + console.info(homeTiming, "[hometiming] tab " + id + " (" + count + + " sections) done at +" + (Date.now() - root._mountT0) + "ms") + } + // Reparsed whenever Rust republishes the JSON documents (one per // Discover tab — phase 13). readonly property var sections: JSON.parse(QbzHome.homeSectionsJson) @@ -589,8 +608,17 @@ Rectangle { id: trackRowComp TrackSlimRow { card: modelData } } + // `active` matters as much as `sourceComponent`: the + // Repeater above is fed the WHOLE item list, and the + // rows past `total` were being built in full and then + // hidden by the delegate's `visible`. A hidden row + // costs exactly as much to construct as a shown one — + // it just never repays it. `sectionData.items` can run + // several times `total` (24), so this is most of the + // grid. Loader { anchors.fill: parent + active: index < sgrid.total sourceComponent: sgrid.tracks ? trackRowComp : albumRowComp } } @@ -1390,10 +1418,46 @@ Rectangle { spacing: 40 // ===== Home tab ========================================== - Column { - id: homeTab - visible: root.activeTab === "home" + // + // ONE TAB EXISTS AT A TIME — and the `Loader` is what makes + // that true. The four tab bodies used to be four sibling + // Columns gated on `visible:`, which hides a Column but does + // NOT stop QML from building it: every Repeater under every + // tab still instantiated every rail and every card. Measured + // on this account (9 + 7 + 12 sections, 423 cards), a Home + // mount spent ~360 of its ~410 ms inside the tab columns, and + // three quarters of those sections belonged to tabs nobody was + // looking at. The proof was the per-tab stamp below firing for + // `forYou` and `editorPicks` while they were invisible. + // + // This is also what the REFERENCE does, and the port had + // simply diverged: HomeView.slint:321 runs ONE repeater whose + // model is picked by a ternary on the active tab, and mounts + // For You / Recommendations behind `if` (:579, :608). Its own + // comment — "the For You tab's lists are pushed empty so this + // repeater renders nothing for it" — is the same rule stated + // from the data side. + // + // SYNCHRONOUS on purpose (`asynchronous` is left at its + // default). Async incubation is time-sliced: it does not + // remove the work, it spreads it over more frames, which is + // why the router-level experiment made the app dramatically + // worse and was reverted the same day. See ContentRouter.qml. + // + // The trade is that switching tabs now builds the tab you + // switch TO, instead of it being pre-built. That is the right + // way round: a mount happens on every navigation into + // Discover, a tab switch only when asked for, and one tab + // costs a quarter of four. + Loader { width: parent.width - 64 + active: root.activeTab === "home" + // Without this the Column reserves a spacing slot for an + // inactive Loader (it is a zero-size but VISIBLE child), + // leaving a 40px gap above the active tab. + visible: active + sourceComponent: Column { + width: parent.width spacing: 40 // Loading skeleton (HomeSkeleton: two shimmer rows). The @@ -1450,30 +1514,44 @@ Rectangle { // Section rails. SectionRails { sectionsModel: root.sections; tabId: "home" } + Component.onCompleted: root._stampTab("home", root.sections.length) + } } // ===== Editor's Picks (phase 13) ======================== - Column { - visible: root.activeTab === "editorPicks" + // Loader-gated for the reason spelled out on the Home tab. + Loader { width: parent.width - 64 - spacing: 40 - TabSkeleton { - visible: QbzHome.homeLoading && root.editorSections.length === 0 - phase: root.skelPhase + active: root.activeTab === "editorPicks" + visible: active + sourceComponent: Column { + width: parent.width + spacing: 40 + TabSkeleton { + visible: QbzHome.homeLoading && root.editorSections.length === 0 + phase: root.skelPhase + } + SectionRails { sectionsModel: root.editorSections; tabId: "editorPicks" } + Component.onCompleted: root._stampTab("editorPicks", root.editorSections.length) } - SectionRails { sectionsModel: root.editorSections; tabId: "editorPicks" } } // ===== For You (phase 13) ================================= - Column { - visible: root.activeTab === "forYou" + // Loader-gated for the reason spelled out on the Home tab. + Loader { width: parent.width - 64 - spacing: 40 - TabSkeleton { - visible: QbzHome.homeLoading && root.forYouSections.length === 0 - phase: root.skelPhase + active: root.activeTab === "forYou" + visible: active + sourceComponent: Column { + width: parent.width + spacing: 40 + TabSkeleton { + visible: QbzHome.homeLoading && root.forYouSections.length === 0 + phase: root.skelPhase + } + SectionRails { sectionsModel: root.forYouSections; tabId: "forYou" } + Component.onCompleted: root._stampTab("forYou", root.forYouSections.length) } - SectionRails { sectionsModel: root.forYouSections; tabId: "forYou" } } // ===== Recommendations (external reco engine) ============= @@ -1483,44 +1561,52 @@ Rectangle { // later entry repaints from memory / the engine's own result // cache, so reopening Discover costs no external traffic. // - // The load is kicked BOTH on mount and on becoming visible: - // the view is instantiated while Home is the active tab, so a - // visible-only hook would be thrown away on the mount pass and - // a mount-only hook would never fire for a tab switch. - Column { - id: recoTab - visible: root.activeTab === "recommendations" + // Loader-gated like the other three (see the Home tab), and + // the gate SIMPLIFIED the lazy-load hook it used to need. The + // old pair — `Component.onCompleted` plus `onVisibleChanged`, + // both funnelled through a `kick()` that re-checked + // `visible` — existed only because this Column was built + // eagerly while Home was the active tab: the mount fired too + // early to mean anything, so a second hook had to catch the + // tab actually being shown. Now the component does not exist + // until the tab is selected, so mounting IS being selected and + // one `Component.onCompleted` says it exactly. + Loader { width: parent.width - 64 - spacing: 40 - - function kick() { - if (recoTab.visible) - QbzHome.loadRecommendations() - } - Component.onCompleted: recoTab.kick() - onVisibleChanged: recoTab.kick() - - // Same two-row shimmer as the other tabs, while the first - // build is in flight and nothing has painted yet. - TabSkeleton { - visible: QbzHome.recoLoading && root.recoSections.length === 0 - phase: root.skelPhase - } - - // Rails resolve progressively — a row that is still - // building, or whose service is not connected, is simply - // ABSENT from the document (never an empty frame). - SectionRails { sectionsModel: root.recoSections; tabId: "recommendations" } - - // Nothing built and nothing in flight: the Slint empty - // state, verbatim msgids. - QbzEmptyState { - visible: !QbzHome.recoLoading && root.recoSections.length === 0 + active: root.activeTab === "recommendations" + visible: active + sourceComponent: Column { width: parent.width - title: QbzSession.tr("No recommendations yet", QbzSession.trRev) - body: QbzSession.tr("Connect Last.fm or ListenBrainz in Settings, or play more music, to get personalized recommendations.", QbzSession.trRev) - actionLabel: QbzSession.tr("Open Settings", QbzSession.trRev) - onActionClicked: QbzShell.navigateTo("settings") + spacing: 40 + + // Every later entry repaints from memory / the + // engine's own result cache, so re-selecting the tab + // costs no external traffic even though the component + // is rebuilt. + Component.onCompleted: QbzHome.loadRecommendations() + + // Same two-row shimmer as the other tabs, while the + // first build is in flight and nothing has painted yet. + TabSkeleton { + visible: QbzHome.recoLoading && root.recoSections.length === 0 + phase: root.skelPhase + } + + // Rails resolve progressively — a row that is still + // building, or whose service is not connected, is + // simply ABSENT from the document (never an empty frame). + SectionRails { sectionsModel: root.recoSections; tabId: "recommendations" } + + // Nothing built and nothing in flight: the Slint empty + // state, verbatim msgids. + QbzEmptyState { + visible: !QbzHome.recoLoading && root.recoSections.length === 0 + width: parent.width + title: QbzSession.tr("No recommendations yet", QbzSession.trRev) + body: QbzSession.tr("Connect Last.fm or ListenBrainz in Settings, or play more music, to get personalized recommendations.", QbzSession.trRev) + actionLabel: QbzSession.tr("Open Settings", QbzSession.trRev) + onActionClicked: QbzShell.navigateTo("settings") + } } } } diff --git a/crates/qbz-qt/qml/views/LocalLibraryView.qml b/crates/qbz-qt/qml/views/LocalLibraryView.qml index dde3c73f9..8afd05190 100644 --- a/crates/qbz-qt/qml/views/LocalLibraryView.qml +++ b/crates/qbz-qt/qml/views/LocalLibraryView.qml @@ -812,25 +812,37 @@ Rectangle { onActionClicked: QbzShell.navigateTo("settings") } - LocalAlbumsTab { + // ONE TAB EXISTS AT A TIME. These four were sibling mounts gated + // on `visible:`, which hides an item but does not stop QML from + // building it — so every entry into Local Library instantiated all + // four tab bodies and their views' first screenful of delegates, + // to show one. The same divergence measured in HomeView.qml (see + // the Home tab there for the numbers and for why `asynchronous` is + // NOT the answer); the Tracks tab is the one that makes it matter + // here, being the 16K-row view. + // + // `active` also carries the tab's own precondition: with no local + // library there is nothing to build, and the empty state above is + // what shows instead. + Loader { anchors.fill: parent - visible: QbzLocal.localAvailable && root.activeTab === "albums" - view: root + active: QbzLocal.localAvailable && root.activeTab === "albums" + sourceComponent: LocalAlbumsTab { view: root } } - LocalArtistsTab { + Loader { anchors.fill: parent - visible: QbzLocal.localAvailable && root.activeTab === "artists" - view: root + active: QbzLocal.localAvailable && root.activeTab === "artists" + sourceComponent: LocalArtistsTab { view: root } } - LocalFoldersTab { + Loader { anchors.fill: parent - visible: QbzLocal.localAvailable && root.activeTab === "folders" - view: root + active: QbzLocal.localAvailable && root.activeTab === "folders" + sourceComponent: LocalFoldersTab { view: root } } - LocalTracksTab { + Loader { anchors.fill: parent - visible: QbzLocal.localAvailable && root.activeTab === "tracks" - view: root + active: QbzLocal.localAvailable && root.activeTab === "tracks" + sourceComponent: LocalTracksTab { view: root } } } }