mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
perf(desktop): stop the Projects fan refetching on re-entry and running after leave (#6458)
Entering Projects fires a large fan: an exhaustive paginated relay enumeration (projects/repos/tombstones), five 2,000-event work-item queries plus assignment-operation scans, per-repo activity summaries, and a local-repository filesystem scan. Measured on a large community (101 issues / 258 PRs): | query | measured cost | |---|---| | work-items (5 × 2,000-event REQs + assignment scans) | 3.5–3.9s | | activity summaries | 4.1s | | repository activity | 1.0–2.2s | | local repository scan | 1.7s | Two lifecycle bugs made the fan far more expensive than it needs to be: - **Freshness windows guaranteed a full refetch on nearly every re-entry** (60s enumeration, 30s work-items/activity, 10s local scan) — i.e., the costs above were re-paid on almost every visit. Every local write path already invalidates its keys explicitly (issue/PR mutations, project creation, repo sync), so the short windows only served remote-actor freshness. Raised to 5m/2m/2m with a 30m enumeration cache: re-entries now paint from cache, and the fan re-runs at most every 2–5 minutes. - **Leaving Projects left the whole fan running**, competing with the next surface's channel fetches on the same relay connection. AbortSignal is now threaded through the enumeration and assignment pagination loops (optional params — behavior identical without a signal), and leaving the surface cancels the work-items query. Deliberately NOT cancelled: the enumeration (the always-mounted sidebar projects section observes it and its 30m cache is valuable), repo snapshots and local scans (native work that can't abort — cancelling would discard the finished result and force the same clones again), and activity summaries (a single bounded request). Abort behavior is covered by red-first unit tests on both pagination loops. Remaining follow-up (out of scope): the queries themselves want a relay-side aggregate instead of shipping thousands of events to compute counts client-side. --------- Signed-off-by: Max Lampert <maxwell@squareup.com>
This commit is contained in:
@@ -285,3 +285,38 @@ test("a failed assignment query surfaces as a failed section instead of silent l
|
||||
|
||||
assert.ok(result.issues.failedSections.includes("assignments"));
|
||||
});
|
||||
|
||||
test("fetchAssignmentOperationEvents stops paginating once its signal aborts", async () => {
|
||||
// A permanently-full page would paginate forever without the cursor; abort
|
||||
// after the first page and require the loop to stop with AbortError.
|
||||
const controller = new AbortController();
|
||||
let fetches = 0;
|
||||
const fullPage = (until) =>
|
||||
Array.from({ length: 500 }, (_, index) => ({
|
||||
id: `${until ?? "head"}-${index}`.padEnd(64, "0"),
|
||||
kind: 1,
|
||||
pubkey: "a".repeat(64),
|
||||
created_at: 1_000_000 - fetches * 1_000 - index,
|
||||
content: JSON.stringify({ type: "assign" }),
|
||||
tags: [],
|
||||
}));
|
||||
const fetchEvents = async (filter) => {
|
||||
fetches += 1;
|
||||
// Bound the fake: without the abort support the loop would paginate
|
||||
// forever (each page has a fresh cursor) and OOM the test run.
|
||||
if (fetches > 3) throw new Error("kept paginating after abort");
|
||||
const page = fullPage(filter.until);
|
||||
controller.abort();
|
||||
return page;
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
fetchAssignmentOperationEvents(
|
||||
["issue".padEnd(64, "1")],
|
||||
fetchEvents,
|
||||
controller.signal,
|
||||
),
|
||||
(error) => error.name === "AbortError",
|
||||
);
|
||||
assert.equal(fetches, 1);
|
||||
});
|
||||
|
||||
@@ -68,6 +68,7 @@ export async function fetchAssignmentOperationEvents(
|
||||
fetchEvents: (
|
||||
filter: FetchEventsInput,
|
||||
) => Promise<RelayEvent[]> = relayClient.fetchEvents.bind(relayClient),
|
||||
signal?: AbortSignal,
|
||||
): Promise<RelayEvent[]> {
|
||||
if (issueIds.length === 0) return [];
|
||||
const chunks: string[][] = [];
|
||||
@@ -75,7 +76,9 @@ export async function fetchAssignmentOperationEvents(
|
||||
chunks.push(issueIds.slice(i, i + ISSUE_ID_CHUNK_SIZE));
|
||||
}
|
||||
const pages = await Promise.all(
|
||||
chunks.map((chunk) => fetchIssueCommentsExhaustively(chunk, fetchEvents)),
|
||||
chunks.map((chunk) =>
|
||||
fetchIssueCommentsExhaustively(chunk, fetchEvents, signal),
|
||||
),
|
||||
);
|
||||
const seen = new Map<string, RelayEvent>();
|
||||
for (const page of pages) {
|
||||
@@ -91,11 +94,15 @@ export async function fetchAssignmentOperationEvents(
|
||||
async function fetchIssueCommentsExhaustively(
|
||||
issueIds: string[],
|
||||
fetchEvents: (filter: FetchEventsInput) => Promise<RelayEvent[]>,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RelayEvent[]> {
|
||||
const seen = new Map<string, RelayEvent>();
|
||||
let limit = ASSIGNMENT_PAGE_LIMIT;
|
||||
let until: number | undefined;
|
||||
for (;;) {
|
||||
// Leaving the Projects surface cancels its queries; stop queuing pages
|
||||
// behind the next surface's fetches.
|
||||
signal?.throwIfAborted();
|
||||
const page = await fetchEvents({
|
||||
kinds: [KIND_TEXT_NOTE],
|
||||
"#e": issueIds,
|
||||
|
||||
@@ -168,13 +168,18 @@ export function eventToProject(
|
||||
}
|
||||
|
||||
export async function fetchProjects(
|
||||
fetchExhaustively: FetchProjectEventsExhaustively = fetchProjectEventsExhaustively,
|
||||
fetchExhaustively?: FetchProjectEventsExhaustively,
|
||||
signal?: AbortSignal,
|
||||
): Promise<Project[]> {
|
||||
// Delegates to `buildProjectsFromFetcher` in `projectEnumeration.ts`, which
|
||||
// is the pure, Tauri-free core of this operation. That helper's javadoc
|
||||
// explains the fail-closed tombstone contract and the NIP-OA owner-deletion
|
||||
// relay-side-suppression decision.
|
||||
return buildProjectsFromFetcher(fetchExhaustively, {
|
||||
const fetcher: FetchProjectEventsExhaustively =
|
||||
fetchExhaustively ??
|
||||
((kinds, extraFilter) =>
|
||||
fetchProjectEventsExhaustively(kinds, extraFilter, undefined, signal));
|
||||
return buildProjectsFromFetcher(fetcher, {
|
||||
relayOrigin: getCachedRelayOrigin(),
|
||||
hiddenAddresses: new Set(readHiddenProjectCards()),
|
||||
});
|
||||
@@ -637,22 +642,44 @@ async function deleteProject(project: Project): Promise<void> {
|
||||
|
||||
export const projectsQueryKey = ["projects"] as const;
|
||||
|
||||
/**
|
||||
* Freshness windows for the Projects surface. Every local write path
|
||||
* invalidates its keys explicitly (issue/PR mutations, project creation,
|
||||
* repo sync), so short windows bought nothing for own-actions and charged a
|
||||
* full relay fan-out — project enumeration is an exhaustive paginated scan,
|
||||
* work items are five 2,000-event queries — on nearly every Projects
|
||||
* re-entry. Remote actors' changes surface within the window. gcTime keeps
|
||||
* the enumeration cached across visits so re-entering Projects paints from
|
||||
* cache instead of blocking on the scan.
|
||||
*/
|
||||
export const PROJECTS_STALE_TIME_MS = 5 * 60_000;
|
||||
// Window-guarded like react-query's own server default (Infinity): an
|
||||
// explicit finite gcTime schedules a real, non-unref'd timeout per cache
|
||||
// entry, which keeps node test processes alive for the full 30 minutes.
|
||||
export const PROJECTS_GC_TIME_MS =
|
||||
typeof window === "undefined" ? Number.POSITIVE_INFINITY : 30 * 60_000;
|
||||
export const PROJECT_WORK_ITEMS_STALE_TIME_MS = 2 * 60_000;
|
||||
export const PROJECT_ACTIVITY_STALE_TIME_MS = 2 * 60_000;
|
||||
export const PROJECT_LOCAL_REPOS_STALE_TIME_MS = 2 * 60_000;
|
||||
|
||||
export function useProjectsQuery() {
|
||||
return useQuery({
|
||||
queryKey: projectsQueryKey,
|
||||
queryFn: () => fetchProjects(),
|
||||
staleTime: 60_000,
|
||||
queryFn: ({ signal }) => fetchProjects(undefined, signal),
|
||||
staleTime: PROJECTS_STALE_TIME_MS,
|
||||
gcTime: PROJECTS_GC_TIME_MS,
|
||||
});
|
||||
}
|
||||
|
||||
export function useProjectQuery(projectId: string) {
|
||||
return useQuery({
|
||||
queryKey: projectsQueryKey,
|
||||
queryFn: () => fetchProjects(),
|
||||
queryFn: ({ signal }) => fetchProjects(undefined, signal),
|
||||
select: (projects) =>
|
||||
projects.find((project) => projectMatchesRouteId(project, projectId)) ??
|
||||
null,
|
||||
staleTime: 60_000,
|
||||
staleTime: PROJECTS_STALE_TIME_MS,
|
||||
gcTime: PROJECTS_GC_TIME_MS,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -793,7 +820,8 @@ export function useProjectLocalRepositoriesQuery(reposDir?: string | null) {
|
||||
return useQuery({
|
||||
queryKey: ["projects", "local-repositories", reposDir ?? "default"],
|
||||
queryFn: () => listProjectLocalRepositories({ reposDir }),
|
||||
staleTime: 10_000,
|
||||
// Filesystem scan; repo sync/clone flows invalidate this key on change.
|
||||
staleTime: PROJECT_LOCAL_REPOS_STALE_TIME_MS,
|
||||
retry: 1,
|
||||
});
|
||||
}
|
||||
@@ -828,9 +856,22 @@ export function useProjectPullRequestsQuery(
|
||||
export function useProjectsWorkItemsQuery(projects: Project[]) {
|
||||
return useQuery({
|
||||
enabled: projects.length > 0,
|
||||
queryKey: ["projects", "work-items", projects.map((project) => project.id)],
|
||||
queryFn: () => fetchProjectsWorkItems(projects),
|
||||
staleTime: 30_000,
|
||||
queryKey: [
|
||||
"projects",
|
||||
"work-items",
|
||||
projects.map((project) => project.id),
|
||||
// Repo attach/detach changes the fan-out inputs without changing
|
||||
// project ids; keying on addresses too prevents a pre-attach result
|
||||
// from serving as fresh for the whole staleTime window.
|
||||
projects
|
||||
.flatMap((project) =>
|
||||
project.repositories.map((repository) => repository.repoAddress),
|
||||
)
|
||||
.sort(),
|
||||
],
|
||||
queryFn: ({ signal }) =>
|
||||
fetchProjectsWorkItems(projects, undefined, signal),
|
||||
staleTime: PROJECT_WORK_ITEMS_STALE_TIME_MS,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -935,7 +976,7 @@ export function useProjectActivitySummariesQuery(projects: Project[]) {
|
||||
enabled: repoAddresses.length > 0,
|
||||
queryKey: ["projects", "activity-summaries", repoAddresses],
|
||||
queryFn: () => fetchProjectActivitySummaries(projects),
|
||||
staleTime: 30_000,
|
||||
staleTime: PROJECT_ACTIVITY_STALE_TIME_MS,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -245,3 +245,31 @@ test("buildProjectsFromFetcher still suppresses deleted heads via the scoped fet
|
||||
const projects = await buildProjectsFromFetcher(fetchExhaustively);
|
||||
assert.deepEqual(projects, [], "deleted repo must not surface as a project");
|
||||
});
|
||||
|
||||
test("enumerateProjectEvents stops paginating once its signal aborts", async () => {
|
||||
// 3 full pages of 2 → without an abort the enumeration would fetch all of
|
||||
// them plus boundary drains. Abort after the first page: the loop must
|
||||
// throw before requesting another page.
|
||||
const events = [
|
||||
relayEvent("a", 1_000),
|
||||
relayEvent("b", 900),
|
||||
relayEvent("c", 800),
|
||||
relayEvent("d", 700),
|
||||
relayEvent("e", 600),
|
||||
relayEvent("f", 500),
|
||||
];
|
||||
const controller = new AbortController();
|
||||
let pageFetches = 0;
|
||||
const fetchPage = async (filter) => {
|
||||
pageFetches += 1;
|
||||
const page = await fetcherFor(events)(filter);
|
||||
controller.abort();
|
||||
return page;
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
enumerateProjectEvents(fetchPage, [30617], 2, undefined, controller.signal),
|
||||
(error) => error.name === "AbortError",
|
||||
);
|
||||
assert.equal(pageFetches, 1);
|
||||
});
|
||||
|
||||
@@ -44,6 +44,7 @@ export async function enumerateProjectEvents(
|
||||
kinds: number[],
|
||||
pageSize: number,
|
||||
extraFilter?: ProjectEventExtraFilter,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RelayEvent[]> {
|
||||
if (!Number.isSafeInteger(pageSize) || pageSize <= 0) {
|
||||
throw new Error(
|
||||
@@ -55,6 +56,10 @@ export async function enumerateProjectEvents(
|
||||
let until: number | undefined;
|
||||
|
||||
for (;;) {
|
||||
// Each relay REQ is bounded, but this loop is not: leaving the Projects
|
||||
// surface cancels its queries, and the abort must stop the enumeration
|
||||
// from queuing further pages behind the next surface's fetches.
|
||||
signal?.throwIfAborted();
|
||||
const page = await fetchPage({
|
||||
...extraFilter,
|
||||
kinds,
|
||||
@@ -65,6 +70,7 @@ export async function enumerateProjectEvents(
|
||||
if (page.length < pageSize) return [...eventsById.values()];
|
||||
|
||||
const oldest = Math.min(...page.map((event) => event.created_at));
|
||||
signal?.throwIfAborted();
|
||||
const boundary = await fetchPage({
|
||||
...extraFilter,
|
||||
kinds,
|
||||
@@ -94,12 +100,14 @@ export function fetchProjectEventsExhaustively(
|
||||
kinds: number[],
|
||||
extraFilter?: ProjectEventExtraFilter,
|
||||
pageSize = PROJECT_ENUMERATION_PAGE_SIZE,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RelayEvent[]> {
|
||||
return enumerateProjectEvents(
|
||||
(filter) => relayClient.fetchEvents(filter),
|
||||
kinds,
|
||||
pageSize,
|
||||
extraFilter,
|
||||
signal,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -82,6 +82,7 @@ export async function fetchProjectsWorkItems<TProject extends ProjectReference>(
|
||||
fetchEvents: (
|
||||
filter: FetchEventsInput,
|
||||
) => Promise<RelayEvent[]> = relayClient.fetchEvents.bind(relayClient),
|
||||
signal?: AbortSignal,
|
||||
): Promise<ProjectsWorkItemsResult<TProject>> {
|
||||
const repoAddresses = [
|
||||
...new Set(
|
||||
@@ -129,10 +130,17 @@ export async function fetchProjectsWorkItems<TProject extends ProjectReference>(
|
||||
.filter((event) => event.kind === KIND_GIT_ISSUE)
|
||||
.map((event) => event.id),
|
||||
fetchEvents,
|
||||
signal,
|
||||
),
|
||||
),
|
||||
]);
|
||||
|
||||
// The five eager queries above are single bounded REQs the relay client
|
||||
// cannot abort mid-flight; only the assignment pagination is abort-aware.
|
||||
// What cancellation CAN save here is the reduce work below and caching a
|
||||
// result for a surface the user already left.
|
||||
signal?.throwIfAborted();
|
||||
|
||||
if (rootResult.status === "rejected") {
|
||||
throw rootResult.reason instanceof Error
|
||||
? rootResult.reason
|
||||
|
||||
@@ -3,6 +3,7 @@ import * as React from "react";
|
||||
|
||||
import {
|
||||
fetchRepositoryActivitySummaries,
|
||||
PROJECT_ACTIVITY_STALE_TIME_MS,
|
||||
type Project,
|
||||
} from "@/features/projects/hooks";
|
||||
|
||||
@@ -27,6 +28,6 @@ export function useRepositoryActivitySummariesQuery(projects: Project[]) {
|
||||
enabled: repoAddresses.length > 0,
|
||||
queryKey: ["projects", "activity-summaries", "repositories", repoAddresses],
|
||||
queryFn: () => fetchRepositoryActivitySummaries(repositories),
|
||||
staleTime: 30_000,
|
||||
staleTime: PROJECT_ACTIVITY_STALE_TIME_MS,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,6 +1,30 @@
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import * as React from "react";
|
||||
|
||||
import { ProjectsView } from "@/features/projects/ui/ProjectsView";
|
||||
|
||||
export function ProjectsScreen() {
|
||||
const queryClient = useQueryClient();
|
||||
React.useEffect(() => {
|
||||
return () => {
|
||||
// Leaving the surface: stop the work-items fan. Its assignment-operation
|
||||
// pagination is abort-aware and unbounded, so cancellation genuinely
|
||||
// stops relay work that would otherwise compete with the next surface's
|
||||
// channel fetches. Cached data stays; the next visit refetches per
|
||||
// staleTime.
|
||||
//
|
||||
// Deliberately NOT cancelled:
|
||||
// - projects enumeration: the always-mounted sidebar projects section
|
||||
// observes this key, so it must keep filling its 30-minute cache.
|
||||
// - repo snapshots / local repositories: native clone/scan work cannot
|
||||
// be aborted mid-flight; cancellation would only discard the finished
|
||||
// result and force the same clones again on the next visit.
|
||||
// - activity summaries: a single bounded request — it completes either
|
||||
// way, so cancelling just wastes the response.
|
||||
void queryClient.cancelQueries({ queryKey: ["projects", "work-items"] });
|
||||
};
|
||||
}, [queryClient]);
|
||||
|
||||
return (
|
||||
<div className="relative flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden">
|
||||
<ProjectsView />
|
||||
|
||||
@@ -63,6 +63,9 @@ test("relay invalidation separates relay project queries from local repo work",
|
||||
["project", "project-1", "pull-requests"],
|
||||
["projects", "issues", ["project-1"]],
|
||||
["projects", "activity-summaries", ["addr-1"]],
|
||||
// Work items fan out over the relay and are fresh for two minutes;
|
||||
// reconnect auto-heal must be able to repair a partial result.
|
||||
["projects", "work-items", ["project-1"], ["addr-1"]],
|
||||
]) {
|
||||
assert.equal(isRelayDependentQueryKey(queryKey), true, queryKey.join("/"));
|
||||
}
|
||||
|
||||
@@ -39,6 +39,10 @@ const RELAY_PROJECT_QUERY_PARTS = new Set<string>([
|
||||
"activity-summaries",
|
||||
"issues",
|
||||
"pull-requests",
|
||||
// Fresh for two minutes (PROJECT_WORK_ITEMS_STALE_TIME_MS) and tolerant of
|
||||
// partial fan-out failure — reconnect auto-heal is what repairs a partial
|
||||
// result inside that window.
|
||||
"work-items",
|
||||
]);
|
||||
|
||||
const LOCAL_PROJECT_QUERY_PARTS = new Set<string>([
|
||||
|
||||
Reference in New Issue
Block a user