mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
Optimize feed lifecycle and navigation rendering
This commit is contained in:
1 parent
2c4aefe872
commit
687f8c438d
12 files changed
+287
-21
No files matched your search
@@ -1,6 +1,6 @@
|
||||
# BitVid Performance & Efficiency Plan
|
||||
|
||||
Status: **Stage 2 in progress — startup hardening validated locally; recommendation indexes now activate on demand**
|
||||
Status: **Stage 2 complete locally — recommendation indexes are demand-driven and coalesced; Stage 3 source-health cancellation audited next**
|
||||
|
||||
## Goal
|
||||
|
||||
@@ -173,13 +173,14 @@ old diagnostic behavior.
|
||||
|
||||
## Stage 2 — Lazy recommendation indexes
|
||||
|
||||
Implemented first slice: `ExploreDataService` is no longer created during
|
||||
Implemented: `ExploreDataService` is no longer created during
|
||||
ordinary bootstrap. Home/Profile sessions therefore create no recommendation
|
||||
worker or recommendation intervals. Opening For You or Explore creates and
|
||||
activates the service; events received while inactive mark it dirty and are
|
||||
included in the activation refresh. Focused lifecycle, feed-coordinator, and
|
||||
build checks pass; browser validation of both recommendation views remains the
|
||||
next gate before further Stage 2 coalescing work.
|
||||
included in the activation refresh. Concurrent history/IDF signals share one
|
||||
running rebuild and retain one forced follow-up pass. Focused lifecycle,
|
||||
feed-coordinator, production-build, and browser navigation checks pass; manual
|
||||
For You/Explore validation confirmed normal recommendations and responsive Home.
|
||||
|
||||
### Change
|
||||
|
||||
@@ -224,6 +225,24 @@ eager initialization if feed freshness regresses.
|
||||
|
||||
## Stage 3 — Bound source-health work to visible cards
|
||||
|
||||
### Audit findings (2026-07-26)
|
||||
|
||||
- `createCardObserver` retains an `IntersectionObserver` per grid container but
|
||||
has no disposal API. `VideoListView.render()` replaces card DOM without
|
||||
disconnecting the old observer, so detached cards can stay observed.
|
||||
- `gridHealth` has a globally bounded, priority-ordered torrent queue, but its
|
||||
queued jobs have no owner/cancellation handle. A discarded card can therefore
|
||||
still start a real tracker/WebRTC probe.
|
||||
- URL health uses an independent observer and video-probe wait queue with the
|
||||
same missing ownership boundary. Its fetch timeout aborts only on timeout,
|
||||
not when the card/grid disappears.
|
||||
|
||||
Implementation order: add disposable observer registrations and call them from
|
||||
grid replacement/unmount; associate queued jobs with card owners and prune jobs
|
||||
when their last owner disappears; finally pass an abort signal into active URL
|
||||
and torrent probes. Cache/in-flight deduplication stays shared so one visible
|
||||
card never cancels work still needed by another visible card.
|
||||
|
||||
### Change
|
||||
|
||||
Keep the existing viewport/preload observer and current two-at-a-time torrent
|
||||
|
||||
+29
-1
@@ -26,6 +26,7 @@ export function createCardObserver(options = {}) {
|
||||
prepareEntries,
|
||||
onCardVisible,
|
||||
onCardRegister,
|
||||
onCardUnregister,
|
||||
createState,
|
||||
} = options;
|
||||
|
||||
@@ -37,7 +38,9 @@ export function createCardObserver(options = {}) {
|
||||
return state;
|
||||
}
|
||||
|
||||
const observedCards = new WeakSet();
|
||||
// A Set is intentional here: disposal must enumerate every card so the
|
||||
// browser observer cannot retain detached feed nodes after a rerender.
|
||||
const observedCards = new Set();
|
||||
const customState = normalizeState(
|
||||
typeof createState === "function" ? createState(container) : {}
|
||||
);
|
||||
@@ -201,9 +204,34 @@ export function createCardObserver(options = {}) {
|
||||
return state ? state.customState : null;
|
||||
}
|
||||
|
||||
function disconnect(container) {
|
||||
const state = containerState.get(container);
|
||||
if (!state) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const card of state.observedCards) {
|
||||
if (typeof onCardUnregister === "function") {
|
||||
try {
|
||||
onCardUnregister({
|
||||
card,
|
||||
state: state.customState,
|
||||
context: { container: state.container, observer: state.observer },
|
||||
});
|
||||
} catch (err) {
|
||||
userLogger.warn("[cardObserver] onCardUnregister failed", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
state.observedCards.clear();
|
||||
state.observer.disconnect();
|
||||
containerState.delete(container);
|
||||
}
|
||||
|
||||
return {
|
||||
observe,
|
||||
refresh,
|
||||
getState,
|
||||
disconnect,
|
||||
};
|
||||
}
|
||||
@@ -10,7 +10,7 @@ import { markAsNormalized } from "./utils.js";
|
||||
// re-runs the feed on the metric cache's debounced change signal).
|
||||
// - Trending (#27): runtime.getViewCount (viewCounter cache)
|
||||
// - Most Zapped (#47): runtime.getZapTotal (zapTotals cache)
|
||||
function createCountRankedSorter(resolveMetric) {
|
||||
function createCountRankedSorter(resolveMetric, { spreadByAuthor = true } = {}) {
|
||||
return function countRankedSorter(items = [], context = {}) {
|
||||
if (!Array.isArray(items)) {
|
||||
return [];
|
||||
@@ -54,9 +54,12 @@ function createCountRankedSorter(resolveMetric) {
|
||||
|
||||
const live = items.filter((entry) => !isMuted(entry));
|
||||
const muted = items.filter((entry) => isMuted(entry));
|
||||
// Rank by the metric, then spread authors so the tab isn't walls of one
|
||||
// creator. The top-ranked item is still chosen first, so it stays topmost.
|
||||
return [...spreadAuthors(live.sort(compare)), ...muted.sort(compare)];
|
||||
const rankedLive = live.sort(compare);
|
||||
// Trending benefits from a little creator diversity. Most Zapped is a
|
||||
// literal leaderboard, so it must retain strict total ordering instead of
|
||||
// moving a larger zap total below a smaller one to avoid repeat authors.
|
||||
const orderedLive = spreadByAuthor ? spreadAuthors(rankedLive) : rankedLive;
|
||||
return [...orderedLive, ...muted.sort(compare)];
|
||||
};
|
||||
}
|
||||
|
||||
@@ -66,7 +69,10 @@ export function createTrendingSorter() {
|
||||
|
||||
// "Most Zapped" (#47): same ranking mechanics over sats totals.
|
||||
export function createMostZappedSorter() {
|
||||
return createCountRankedSorter((context) => context?.runtime?.getZapTotal);
|
||||
return createCountRankedSorter(
|
||||
(context) => context?.runtime?.getZapTotal,
|
||||
{ spreadByAuthor: false },
|
||||
);
|
||||
}
|
||||
|
||||
// Round-robin interleave by author: one video per creator per pass, each pass in
|
||||
|
||||
+2
-1
@@ -573,10 +573,11 @@ const gridCardObserver = createCardObserver({
|
||||
|
||||
export function attachHealthBadges(container, options = {}) {
|
||||
if (!(container instanceof HTMLElement)) {
|
||||
return;
|
||||
return () => {};
|
||||
}
|
||||
if (options && typeof options.onUpdate === "function") {
|
||||
badgeUpdateListeners.add(options.onUpdate);
|
||||
}
|
||||
gridCardObserver.observe(container);
|
||||
return () => gridCardObserver.disconnect(container);
|
||||
}
|
||||
@@ -148,6 +148,10 @@ export default class ExploreDataService {
|
||||
this.active = false;
|
||||
this.historyDirty = true;
|
||||
this.idfDirty = true;
|
||||
this.historyRefreshInFlight = null;
|
||||
this.idfRefreshInFlight = null;
|
||||
this.historyRefreshFollowUp = null;
|
||||
this.idfRefreshFollowUp = null;
|
||||
}
|
||||
|
||||
initialize({ active = false } = {}) {
|
||||
@@ -274,7 +278,33 @@ export default class ExploreDataService {
|
||||
}, this.refreshDebounceMs);
|
||||
}
|
||||
|
||||
async refreshWatchHistoryTagCounts({ force = false, reason } = {}) {
|
||||
async refreshWatchHistoryTagCounts(options = {}) {
|
||||
if (this.historyRefreshInFlight) {
|
||||
// Keep only one fresh pass after the current worker task. Rebuilding for
|
||||
// every relay/event/timer signal is exactly the burst this lifecycle is
|
||||
// intended to prevent.
|
||||
this.historyRefreshFollowUp = {
|
||||
force: true,
|
||||
reason: options.reason || "coalesced",
|
||||
};
|
||||
return this.historyRefreshInFlight;
|
||||
}
|
||||
|
||||
const run = this._refreshWatchHistoryTagCounts(options);
|
||||
this.historyRefreshInFlight = Promise.resolve(run).finally(() => {
|
||||
this.historyRefreshInFlight = null;
|
||||
const followUp = this.historyRefreshFollowUp;
|
||||
this.historyRefreshFollowUp = null;
|
||||
if (followUp && this.active) {
|
||||
void this.refreshWatchHistoryTagCounts(followUp);
|
||||
} else if (followUp) {
|
||||
this.historyDirty = true;
|
||||
}
|
||||
});
|
||||
return this.historyRefreshInFlight;
|
||||
}
|
||||
|
||||
async _refreshWatchHistoryTagCounts({ force = false, reason } = {}) {
|
||||
const now = Date.now();
|
||||
if (!force && this.watchHistoryTagCountsUpdatedAt) {
|
||||
const elapsed = now - this.watchHistoryTagCountsUpdatedAt;
|
||||
@@ -308,7 +338,30 @@ export default class ExploreDataService {
|
||||
}
|
||||
}
|
||||
|
||||
async refreshTagIdf({ force = false, videos, reason } = {}) {
|
||||
async refreshTagIdf(options = {}) {
|
||||
if (this.idfRefreshInFlight) {
|
||||
this.idfRefreshFollowUp = {
|
||||
force: true,
|
||||
reason: options.reason || "coalesced",
|
||||
};
|
||||
return this.idfRefreshInFlight;
|
||||
}
|
||||
|
||||
const run = this._refreshTagIdf(options);
|
||||
this.idfRefreshInFlight = Promise.resolve(run).finally(() => {
|
||||
this.idfRefreshInFlight = null;
|
||||
const followUp = this.idfRefreshFollowUp;
|
||||
this.idfRefreshFollowUp = null;
|
||||
if (followUp && this.active) {
|
||||
void this.refreshTagIdf(followUp);
|
||||
} else if (followUp) {
|
||||
this.idfDirty = true;
|
||||
}
|
||||
});
|
||||
return this.idfRefreshInFlight;
|
||||
}
|
||||
|
||||
async _refreshTagIdf({ force = false, videos, reason } = {}) {
|
||||
const now = Date.now();
|
||||
if (!force && this.tagIdfUpdatedAt) {
|
||||
const elapsed = now - this.tagIdfUpdatedAt;
|
||||
@@ -361,6 +414,8 @@ export default class ExploreDataService {
|
||||
clearTimeout(this.tagIdfRefreshHandle);
|
||||
this.tagIdfRefreshHandle = null;
|
||||
}
|
||||
this.historyRefreshFollowUp = null;
|
||||
this.idfRefreshFollowUp = null;
|
||||
|
||||
this.unsubscribeHandlers.forEach((unsubscribe) => {
|
||||
if (typeof unsubscribe === "function") {
|
||||
|
||||
@@ -166,11 +166,10 @@ export default class ApplicationBootstrap {
|
||||
app.streamHealthSnapshots = new Map();
|
||||
app.boundStreamHealthBadgeHandler = (detail) =>
|
||||
app.handleStreamHealthBadgeUpdate(detail);
|
||||
app.attachHealthBadgesWithCache = (container) => {
|
||||
app.attachHealthBadgesWithCache = (container) =>
|
||||
attachHealthBadges(container, {
|
||||
onUpdate: app.boundStreamHealthBadgeHandler,
|
||||
});
|
||||
};
|
||||
app.defaultModerationSettings = getDefaultModerationSettings();
|
||||
app.moderationSettings = { ...app.defaultModerationSettings };
|
||||
app.relayManager = relayManager;
|
||||
|
||||
@@ -32,6 +32,8 @@ export class VideoListView {
|
||||
(typeof window !== "undefined" ? window : null);
|
||||
this.container = null;
|
||||
this.mediaLoader = mediaLoader;
|
||||
this.healthBadgeDetach = null;
|
||||
this.urlHealthBadgeDetach = null;
|
||||
|
||||
this.badgeHelpers = {
|
||||
attachHealthBadges: badgeHelpers.attachHealthBadges || (() => {}),
|
||||
@@ -236,6 +238,7 @@ export class VideoListView {
|
||||
}
|
||||
|
||||
if (this.container) {
|
||||
this.teardownHealthBadgeObservers();
|
||||
this.container.removeEventListener("click", this._boundClickHandler);
|
||||
}
|
||||
|
||||
@@ -262,6 +265,7 @@ export class VideoListView {
|
||||
|
||||
const markup = this.renderers.getLoadingMarkup(message);
|
||||
if (typeof markup === "string") {
|
||||
this.teardownHealthBadgeObservers();
|
||||
this.container.innerHTML = markup;
|
||||
}
|
||||
}
|
||||
@@ -280,6 +284,16 @@ export class VideoListView {
|
||||
}
|
||||
}
|
||||
|
||||
teardownHealthBadgeObservers() {
|
||||
for (const detach of [this.healthBadgeDetach, this.urlHealthBadgeDetach]) {
|
||||
if (typeof detach === "function") {
|
||||
detach();
|
||||
}
|
||||
}
|
||||
this.healthBadgeDetach = null;
|
||||
this.urlHealthBadgeDetach = null;
|
||||
}
|
||||
|
||||
setPlaybackHandler(handler) {
|
||||
this.handlers.playback = typeof handler === "function" ? handler : null;
|
||||
}
|
||||
@@ -374,6 +388,7 @@ export class VideoListView {
|
||||
return displayVideos;
|
||||
}
|
||||
this.lastRenderedVideoSignature = EMPTY_VIDEO_LIST_SIGNATURE;
|
||||
this.teardownHealthBadgeObservers();
|
||||
this.container.innerHTML = `
|
||||
<p class="flex justify-center items-center h-full w-full text-center text-subtle">
|
||||
No public videos available yet. Be the first to upload one!
|
||||
@@ -413,7 +428,10 @@ export class VideoListView {
|
||||
}));
|
||||
const signature = JSON.stringify(signaturePayload);
|
||||
|
||||
if (signature === this.lastRenderedVideoSignature) {
|
||||
const hasRenderedCards = Boolean(
|
||||
this.container.querySelector('[data-component="video-card"]'),
|
||||
);
|
||||
if (signature === this.lastRenderedVideoSignature && hasRenderedCards) {
|
||||
return displayVideos;
|
||||
}
|
||||
this.lastRenderedVideoSignature = signature;
|
||||
@@ -788,11 +806,12 @@ export class VideoListView {
|
||||
});
|
||||
}
|
||||
|
||||
this.teardownHealthBadgeObservers();
|
||||
this.container.innerHTML = "";
|
||||
this.container.appendChild(fragment);
|
||||
|
||||
this.badgeHelpers.attachHealthBadges(this.container);
|
||||
this.badgeHelpers.attachUrlHealthBadges(this.container, ({ badgeEl, url, eventId }) => {
|
||||
this.healthBadgeDetach = this.badgeHelpers.attachHealthBadges(this.container);
|
||||
this.urlHealthBadgeDetach = this.badgeHelpers.attachUrlHealthBadges(this.container, ({ badgeEl, url, eventId }) => {
|
||||
this.utils.handleUrlHealthBadge({
|
||||
video: this.state.videosMap.get(eventId) || { id: eventId },
|
||||
url,
|
||||
|
||||
@@ -75,12 +75,12 @@ function triggerUrlCheck(card, state) {
|
||||
|
||||
export function attachUrlHealthBadges(container, onCheck) {
|
||||
if (!(container instanceof HTMLElement)) {
|
||||
return;
|
||||
return () => {};
|
||||
}
|
||||
|
||||
const state = urlCardObserver.observe(container);
|
||||
if (state && typeof onCheck === "function") {
|
||||
state.onCheck = onCheck;
|
||||
}
|
||||
return () => urlCardObserver.disconnect(container);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
import { createCardObserver } from "../../js/dom/cardObserver.js";
|
||||
|
||||
test("card observer disconnect unregisters every card and releases its observer", () => {
|
||||
const dom = new JSDOM("<div id=grid><article class='card' data-video-id=a></article><article class='card' data-video-id=b></article></div>");
|
||||
const originalHTMLElement = globalThis.HTMLElement;
|
||||
const originalIntersectionObserver = globalThis.IntersectionObserver;
|
||||
globalThis.HTMLElement = dom.window.HTMLElement;
|
||||
let disconnects = 0;
|
||||
globalThis.IntersectionObserver = class {
|
||||
observe() {}
|
||||
takeRecords() { return []; }
|
||||
disconnect() { disconnects += 1; }
|
||||
};
|
||||
|
||||
const unregistered = [];
|
||||
try {
|
||||
const observer = createCardObserver({
|
||||
onCardUnregister: ({ card }) => unregistered.push(card.dataset.videoId),
|
||||
});
|
||||
const grid = dom.window.document.getElementById("grid");
|
||||
observer.observe(grid);
|
||||
observer.disconnect(grid);
|
||||
|
||||
assert.deepEqual(unregistered.sort(), ["a", "b"]);
|
||||
assert.equal(disconnects, 1);
|
||||
assert.equal(observer.getState(grid), null);
|
||||
} finally {
|
||||
dom.window.close();
|
||||
if (originalHTMLElement === undefined) delete globalThis.HTMLElement;
|
||||
else globalThis.HTMLElement = originalHTMLElement;
|
||||
if (originalIntersectionObserver === undefined) delete globalThis.IntersectionObserver;
|
||||
else globalThis.IntersectionObserver = originalIntersectionObserver;
|
||||
}
|
||||
});
|
||||
@@ -33,3 +33,33 @@ test("ExploreDataService stays idle until a recommendation feed activates it", a
|
||||
assert.equal(idfRefreshes, 1, "inactive feeds do not rebuild recommendation indexes");
|
||||
service.destroy();
|
||||
});
|
||||
|
||||
test("ExploreDataService coalesces a refresh burst into one running pass and one follow-up", async () => {
|
||||
const service = new ExploreDataService({
|
||||
historyRefreshIntervalMs: 0,
|
||||
idfRefreshIntervalMs: 0,
|
||||
});
|
||||
service.active = true;
|
||||
let runs = 0;
|
||||
let releaseFirst;
|
||||
service._refreshTagIdf = async () => {
|
||||
runs += 1;
|
||||
if (runs === 1) {
|
||||
await new Promise((resolve) => {
|
||||
releaseFirst = resolve;
|
||||
});
|
||||
}
|
||||
return new Map();
|
||||
};
|
||||
|
||||
const first = service.refreshTagIdf({ force: true, reason: "initial" });
|
||||
const duplicate = service.refreshTagIdf({ reason: "relay-event" });
|
||||
service.refreshTagIdf({ reason: "timer" });
|
||||
assert.equal(runs, 1, "only one rebuild starts during the burst");
|
||||
|
||||
releaseFirst();
|
||||
await Promise.all([first, duplicate]);
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
assert.equal(runs, 2, "the burst produces exactly one follow-up rebuild");
|
||||
service.destroy();
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
import { VideoListView } from "../../js/ui/views/VideoListView.js";
|
||||
|
||||
test("VideoListView disposes health observers before replacing its card grid", () => {
|
||||
const dom = new JSDOM("<div id=grid></div>");
|
||||
const grid = dom.window.document.getElementById("grid");
|
||||
let healthDetaches = 0;
|
||||
let urlDetaches = 0;
|
||||
const view = new VideoListView({
|
||||
document: dom.window.document,
|
||||
badgeHelpers: {
|
||||
attachHealthBadges: () => () => { healthDetaches += 1; },
|
||||
attachUrlHealthBadges: () => () => { urlDetaches += 1; },
|
||||
},
|
||||
});
|
||||
view.mount(grid);
|
||||
const videos = [{ id: "one", title: "One", pubkey: "a", created_at: 1 }];
|
||||
|
||||
view.render(videos);
|
||||
view.render([{ ...videos[0], title: "Updated", created_at: 2 }]);
|
||||
view.unmount();
|
||||
|
||||
assert.equal(healthDetaches, 2, "rerender and unmount each detach health observers");
|
||||
assert.equal(urlDetaches, 2, "rerender and unmount each detach URL observers");
|
||||
dom.window.close();
|
||||
});
|
||||
|
||||
test("VideoListView restores cards when a navigation remount clears unchanged feed markup", () => {
|
||||
const dom = new JSDOM("<div id=grid></div>");
|
||||
const grid = dom.window.document.getElementById("grid");
|
||||
const view = new VideoListView({ document: dom.window.document });
|
||||
const videos = [{ id: "for-you-one", title: "For You", pubkey: "a", created_at: 1 }];
|
||||
|
||||
view.mount(grid);
|
||||
view.render(videos);
|
||||
assert.equal(grid.querySelectorAll('[data-component="video-card"]').length, 1);
|
||||
|
||||
// Loading a view replaces #videoList. The data can still be identical when
|
||||
// returning to For You, so a signature-only cache must not skip the repaint.
|
||||
grid.innerHTML = '<div data-for-you-empty-state="true"></div>';
|
||||
view.render(videos);
|
||||
|
||||
assert.equal(grid.querySelectorAll('[data-component="video-card"]').length, 1);
|
||||
assert.equal(grid.querySelector('[data-for-you-empty-state]'), null);
|
||||
dom.window.close();
|
||||
});
|
||||
@@ -373,3 +373,25 @@ test("most-zapped sorter: sats desc, recency tie-break, muted sinks", () => {
|
||||
"sats desc, newer wins ties, trusted-muted last regardless of sats",
|
||||
);
|
||||
});
|
||||
|
||||
test("most-zapped sorter keeps strict zap order when one creator has several top videos", () => {
|
||||
const item = (id, author, sats, createdAt) => ({
|
||||
video: { id, pubkey: author, created_at: createdAt },
|
||||
metadata: {},
|
||||
sats,
|
||||
});
|
||||
const items = [
|
||||
item("creator-a-top", "creator-a", 900, 100),
|
||||
item("creator-a-second", "creator-a", 800, 200),
|
||||
item("creator-b", "creator-b", 700, 300),
|
||||
];
|
||||
const sorted = createMostZappedSorter()(items, {
|
||||
runtime: { getZapTotal: (video) => items.find((entry) => entry.video === video)?.sats || 0 },
|
||||
});
|
||||
|
||||
assert.deepEqual(
|
||||
sorted.map((entry) => entry.video.id),
|
||||
["creator-a-top", "creator-a-second", "creator-b"],
|
||||
"a diversity rule must not move a lower-zapped video above a higher-zapped one",
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user