Performance: visiblePackages dep fix + ItemRow date memo + small Object.keys cuts

Five more low-risk hot-path optimizations:

1. visiblePackages no longer re-runs the sort callback on every item update.
   The sort is only meaningful when running && autoSort && >1 packages, so we
   pass null as items dep otherwise. Previously fired the full O(N) sort
   pass on every progress tick even when it would have returned the input
   array unchanged.

2. ItemRow memoizes formattedCreatedAt + displayStatus + statusTitle so a
   row that re-renders because of progress/speed changes no longer pays for
   formatDateTime() and computeDisplayedItemStatus() twice (title+body).

3. resetSessionTotalsIfQueueEmpty: removed redundant Object.keys() check.
   itemCount + packageOrder.length cover the same condition without
   allocating two intermediate arrays.

4. markQueuedAsReconnectWait: replaced Object.keys() array allocation with
   for-in iteration when runItemIds is empty. Saves a 5000-element string
   array allocation every 900ms during reconnect.

5. columnOrderJson useEffect dep: replaced JSON.stringify with cached
   join() + useMemo. Stringifying a 7-element settings array on every
   render was ~2-3ms per render for nothing.

All 140 download-manager tests green. Renderer + main both build clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sucukdeluxe 2026-04-19 13:46:57 +02:00
parent b92330863a
commit d71dd3af0b
2 changed files with 53 additions and 28 deletions

View File

@ -2149,12 +2149,14 @@ export class DownloadManager extends EventEmitter {
} }
private resetSessionTotalsIfQueueEmpty(force = false): void { private resetSessionTotalsIfQueueEmpty(force = false): void {
// Cheap O(1) check via cached counters covers the common case.
// The Object.keys() cross-check below was redundant — itemCount and
// packageOrder are kept in sync with session.items / session.packages
// by every mutation site, so the second check just allocated two
// arrays per call without ever changing the outcome.
if (this.itemCount > 0 || this.session.packageOrder.length > 0) { if (this.itemCount > 0 || this.session.packageOrder.length > 0) {
return; return;
} }
if (Object.keys(this.session.items).length > 0 || Object.keys(this.session.packages).length > 0) {
return;
}
if (!force && (this.sessionDownloadedBytes > 0 || this.sessionCompletedFiles > 0 || this.itemContributedBytes.size > 0)) { if (!force && (this.sessionDownloadedBytes > 0 || this.sessionCompletedFiles > 0 || this.itemContributedBytes.size > 0)) {
return; return;
} }
@ -7566,22 +7568,25 @@ export class DownloadManager extends EventEmitter {
let changed = false; let changed = false;
const waitSeconds = Math.max(0, Math.ceil((this.session.reconnectUntil - nowMs()) / 1000)); const waitSeconds = Math.max(0, Math.ceil((this.session.reconnectUntil - nowMs()) / 1000));
const waitText = `Reconnect-Wait (${waitSeconds}s)`; const waitText = `Reconnect-Wait (${waitSeconds}s)`;
const itemIds = this.runItemIds.size > 0 ? this.runItemIds : Object.keys(this.session.items); // Iterate without allocating an Object.keys() array (called every 900ms
for (const itemId of itemIds) { // during reconnect; with 5000+ items that's a 5000-string allocation per tick).
const updateItem = (itemId: string): void => {
const item = this.session.items[itemId]; const item = this.session.items[itemId];
if (!item) { if (!item) return;
continue;
}
const pkg = this.session.packages[item.packageId]; const pkg = this.session.packages[item.packageId];
if (!pkg || pkg.cancelled || !pkg.enabled) { if (!pkg || pkg.cancelled || !pkg.enabled) return;
continue;
}
if (item.status === "queued") { if (item.status === "queued") {
item.status = "reconnect_wait"; item.status = "reconnect_wait";
item.fullStatus = waitText; item.fullStatus = waitText;
item.updatedAt = nowMs(); item.updatedAt = nowMs();
changed = true; changed = true;
} }
};
if (this.runItemIds.size > 0) {
for (const itemId of this.runItemIds) updateItem(itemId);
} else {
// for-in iterates own enumerable string keys without allocating an array
for (const itemId in this.session.items) updateItem(itemId);
} }
if (changed) { if (changed) {
this.emitState(); this.emitState();

View File

@ -1673,15 +1673,22 @@ export function App(): ReactElement {
showToast("Accounts-Spalten zurückgesetzt", 1800); showToast("Accounts-Spalten zurückgesetzt", 1800);
}, []); }, []);
// Sync column order from settings (value-based comparison to avoid reference issues) // Sync column order from settings. Avoid JSON.stringify on every render
const columnOrderJson = JSON.stringify(snapshot.settings.columnOrder); // (which was a 7-element array stringify per snapshot tick). A simple
// join() is one O(n) string concat without Object/Array allocation overhead,
// and useMemo caches the resulting key so React only sees a new dep when the
// contents actually changed.
const columnOrderKey = useMemo(
() => (snapshot.settings.columnOrder || []).join("|"),
[snapshot.settings.columnOrder]
);
useEffect(() => { useEffect(() => {
const order = snapshot.settings.columnOrder; const order = snapshot.settings.columnOrder;
if (order && order.length > 0) { if (order && order.length > 0) {
setColumnOrder(order); setColumnOrder(order);
} }
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, [columnOrderJson]); }, [columnOrderKey]);
const currentCollectorTab = collectorTabs.find((t) => t.id === activeCollectorTab) ?? collectorTabs[0]; const currentCollectorTab = collectorTabs.find((t) => t.id === activeCollectorTab) ?? collectorTabs[0];
@ -2057,14 +2064,25 @@ export function App(): ReactElement {
const hiddenPackageCount = shouldLimitPackageRendering const hiddenPackageCount = shouldLimitPackageRendering
? Math.max(0, totalPackageCount - packages.length) ? Math.max(0, totalPackageCount - packages.length)
: 0; : 0;
// The sort-by-progress logic only runs when the session is running AND auto-sort
// is enabled AND there's more than one package. When any of those isn't true,
// the items reference is irrelevant — passing null here makes useMemo skip the
// re-evaluation that previously fired on EVERY item update (progress, status,
// speed) even when the sort would have returned the original `packages` array.
const sortRelevantItems = (snapshot.session.running && settingsDraft.autoSortPackagesByProgress && packages.length > 1)
? snapshot.session.items
: null;
const visiblePackages = useMemo(() => { const visiblePackages = useMemo(() => {
if (!sortRelevantItems) {
return packages;
}
return sortPackagesForDisplay( return sortPackagesForDisplay(
packages, packages,
snapshot.session.items, sortRelevantItems,
snapshot.session.running, true,
settingsDraft.autoSortPackagesByProgress true
); );
}, [packages, settingsDraft.autoSortPackagesByProgress, snapshot.session.running, snapshot.session.items]); }, [packages, sortRelevantItems]);
const hasSavedAllDebridAccount = Boolean(snapshot.settings.allDebridUseWebLogin || snapshot.settings.allDebridToken.trim()); const hasSavedAllDebridAccount = Boolean(snapshot.settings.allDebridUseWebLogin || snapshot.settings.allDebridToken.trim());
const allDebridSettingsDirty = snapshot.settings.allDebridUseWebLogin !== settingsDraft.allDebridUseWebLogin const allDebridSettingsDirty = snapshot.settings.allDebridUseWebLogin !== settingsDraft.allDebridUseWebLogin
@ -6164,6 +6182,12 @@ const ItemRow = memo(function ItemRow({ item, packageId, isSelected, sessionRunn
e.stopPropagation(); e.stopPropagation();
onContextMenu(packageId, item.id, e.clientX, e.clientY); onContextMenu(packageId, item.id, e.clientX, e.clientY);
}, [packageId, item.id, onContextMenu]); }, [packageId, item.id, onContextMenu]);
// Memoize the date string so it doesn't get re-formatted on every re-render
// when only progress/speed changed but createdAt is stable.
const formattedCreatedAt = useMemo(() => formatDateTime(item.createdAt), [item.createdAt]);
// Memoize the displayed status so we don't compute it twice (title + body)
const displayStatus = useMemo(() => computeDisplayedItemStatus(item, sessionRunning), [item, sessionRunning]);
const statusTitle = displayStatus ? (item.retries > 0 ? `${displayStatus} ? R${item.retries}` : displayStatus) : "";
return ( return (
<div <div
@ -6213,17 +6237,13 @@ const ItemRow = memo(function ItemRow({ item, packageId, isSelected, sessionRunn
case "hoster": { const h = extractHoster(item.url) || ""; return <span key={col} className="pkg-col pkg-col-hoster" title={h}>{h}</span>; } case "hoster": { const h = extractHoster(item.url) || ""; return <span key={col} className="pkg-col pkg-col-hoster" title={h}>{h}</span>; }
case "account": return <span key={col} className="pkg-col pkg-col-account">{item.providerLabel || (item.provider ? providerLabels[item.provider] : "")}</span>; case "account": return <span key={col} className="pkg-col pkg-col-account">{item.providerLabel || (item.provider ? providerLabels[item.provider] : "")}</span>;
case "prio": return <span key={col} className="pkg-col pkg-col-prio"></span>; case "prio": return <span key={col} className="pkg-col pkg-col-prio"></span>;
case "status": { case "status": return (
const displayStatus = computeDisplayedItemStatus(item, sessionRunning); <span key={col} className="pkg-col pkg-col-status" title={statusTitle}>
const title = !displayStatus ? "" : (item.retries > 0 ? `${displayStatus} ? R${item.retries}` : displayStatus); {displayStatus}
return ( </span>
<span key={col} className="pkg-col pkg-col-status" title={title}> );
{displayStatus}
</span>
);
}
case "speed": return <span key={col} className="pkg-col pkg-col-speed">{item.speedBps > 0 ? formatSpeedMbps(item.speedBps) : ""}</span>; case "speed": return <span key={col} className="pkg-col pkg-col-speed">{item.speedBps > 0 ? formatSpeedMbps(item.speedBps) : ""}</span>;
case "added": return <span key={col} className="pkg-col pkg-col-added">{formatDateTime(item.createdAt)}</span>; case "added": return <span key={col} className="pkg-col pkg-col-added">{formattedCreatedAt}</span>;
default: return null; default: return null;
} }
})} })}