Skip to content

Commit 62ed560

Browse files
amitsingh-007claude
andcommitted
perf: remove redundant reads, refreshes and re-renders
Seven efficiency fixes. No behaviour change intended. shared: - useTaggedBookmarks read and re-parsed the entire bookmarks object 2N+2 times for a person with N tagged bookmarks: getPersonTaggedUrls read it once, then getBookmarkFromHash and getFolderFromHash each read it again per url, then getDefaultOrRootFolderUrls once more. It now reads once and indexes locally. On web that read is a synchronous JSON.parse of the whole DB, so N=50 went from ~102 parses to 1. - getPersonsWithImageUrl called getPersonImageUrls() and caches.open() once per person. Both are hoisted out of the map via a new getBlobUrlFromOpenCache that takes an already-open Cache. The early return when there are no image urls is preserved, so a list still renders when CacheStorage is unavailable rather than rejecting the whole batch. extension: - processPostLogin awaited cachePersonImagesInStorage() then cacheBookmarkFavicons() sequentially. They read different storage items and neither consumes the other's output, so sign-in now costs max() rather than sum() of two network-bound passes. addAllToCache shares one pLimit(20), so peak concurrency is unchanged, and the progress step count stays at 3. - BookmarksPanel re-ran the full context-bookmark filter on every render. The component subscribes to selectedBookmarks/cutBookmarks, so that was once per row click, and the new array identity churned useVirtualizer's getItemKey. Now memoised on [contextBookmarks, searchText]. Same for the web bookmark panel. web: - getAuthIdToken forced a token refresh on every call, and it feeds the httpBatchLink headers() hook, so every tRPC request paid a securetoken round trip even with ~55 mins of validity left. The SDK already refreshes within 5 mins of expiry. The extension already did this correctly. - The upload route buffered the file twice (once to sniff the type, once to compress), up to 5 MB each. It now buffers once. The single-case switch(true) collapses to an if. tests: - sharedBackground was the only shared fixture missing { scope: 'worker' }. background-navigation.spec is describe.serial and every test calls ensureActiveState/clearHistoryStartTime, so it is self-resetting and does not need a profile copy plus Chromium launch per test. Measured on that spec: 17.6s -> 11.4s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d7a4db7 commit 62ed560

9 files changed

Lines changed: 116 additions & 68 deletions

File tree

apps/extension/src/entrypoints/popup/panels/BookmarksPanel/components/BookmarksPanel.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from '@bypass/shared';
99
import { ScrollArea } from '@bypass/ui';
1010
import { useVirtualizer } from '@tanstack/react-virtual';
11-
import { useCallback, useEffect, useRef, useState } from 'react';
11+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
1212
import { useShallow } from 'zustand/react/shallow';
1313

1414
import { MAX_PANEL_SIZE } from '@/constants';
@@ -45,9 +45,12 @@ function BookmarksPanel({ folderId, operation, bmUrl }: BMPanelQueryParams) {
4545
);
4646
const scrollAreaRef = useRef<HTMLDivElement>(null);
4747
const [searchText, setSearchText] = useState('');
48-
const filteredContextBookmarks = getFilteredContextBookmarks(
49-
contextBookmarks,
50-
searchText
48+
// This component re-renders on every row click (selected/cut state), so
49+
// without memoing, the whole list is re-filtered and getItemKey's identity
50+
// churns on each one
51+
const filteredContextBookmarks = useMemo(
52+
() => getFilteredContextBookmarks(contextBookmarks, searchText),
53+
[contextBookmarks, searchText]
5154
);
5255
const virtualizer = useVirtualizer({
5356
count: filteredContextBookmarks.length,

apps/extension/src/entrypoints/popup/panels/HomePopup/utils/sync.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,11 @@ export const processPostLogin = async () => {
6363
// Sync remote firebase to storage
6464
await syncFirebaseToStorage();
6565
incrementProgress(SIGN_IN_TOTAL_STEPS);
66-
// Then do other processes
67-
await cachePersonImagesInStorage();
66+
// Independent network-bound cache warms: person images come from personsItem,
67+
// favicons from bookmarksItem. addAllToCache shares one pLimit(20), so running
68+
// them together costs max() instead of sum() without raising peak concurrency.
69+
await Promise.all([cachePersonImagesInStorage(), cacheBookmarkFavicons()]);
6870
incrementProgress(SIGN_IN_TOTAL_STEPS);
69-
await cacheBookmarkFavicons();
7071
incrementProgress(SIGN_IN_TOTAL_STEPS);
7172
};
7273

apps/extension/tests/fixtures/background-fixture.ts

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,10 @@ const createBackgroundEnv = async (
116116
};
117117
};
118118

119-
export const test = base.extend<{
120-
isolatedBackground: BaseBackgroundEnv;
121-
sharedBackground: BaseBackgroundEnv;
122-
}>({
119+
export const test = base.extend<
120+
{ isolatedBackground: BaseBackgroundEnv },
121+
{ sharedBackground: BaseBackgroundEnv }
122+
>({
123123
async isolatedBackground({}, use, testInfo) {
124124
const userDataDir = await fs.mkdtemp(
125125
path.join(os.tmpdir(), 'chrome-background-profile-')
@@ -142,22 +142,29 @@ export const test = base.extend<{
142142
}
143143
},
144144

145-
async sharedBackground({}, use, testInfo) {
146-
const { browserContext, userDataDir } = await createSharedContext({
147-
headless: testInfo.project.use?.headless ?? true,
148-
});
145+
// Worker-scoped like every other shared fixture: background-navigation.spec
146+
// is describe.serial and each test calls ensureActiveState /
147+
// clearHistoryStartTime, so it is self-resetting and does not need a fresh
148+
// profile copy + Chromium launch per test.
149+
sharedBackground: [
150+
async ({}, use, testInfo) => {
151+
const { browserContext, userDataDir } = await createSharedContext({
152+
headless: testInfo.project.use?.headless ?? true,
153+
});
149154

150-
try {
151-
const backgroundSW = await createSharedBackgroundSW(browserContext);
152-
const extensionId = await getExtensionId(backgroundSW);
153-
const env = await createBackgroundEnv(browserContext, extensionId);
154-
155-
await use(env);
156-
} finally {
157-
await browserContext.close();
158-
await fs.rm(userDataDir, { recursive: true, force: true });
159-
}
160-
},
155+
try {
156+
const backgroundSW = await createSharedBackgroundSW(browserContext);
157+
const extensionId = await getExtensionId(backgroundSW);
158+
const env = await createBackgroundEnv(browserContext, extensionId);
159+
160+
await use(env);
161+
} finally {
162+
await browserContext.close();
163+
await fs.rm(userDataDir, { recursive: true, force: true });
164+
}
165+
},
166+
{ scope: 'worker' },
167+
],
161168
});
162169

163170
export const { expect } = test;
Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { fileTypeFromBuffer } from 'file-type';
22
import sharp from 'sharp';
33

4-
const getCompressedImage = async (file: File) => {
4+
const getCompressedImage = async (buffer: Buffer, fileSize: number) => {
55
return (
6-
sharp(Buffer.from(await file.arrayBuffer()))
6+
sharp(buffer)
77
// When changing this width, change on client app as well
88
.resize({ width: 250, withoutEnlargement: true })
9-
.jpeg({ quality: file.size < 50 * 1024 ? 100 : 90 })
9+
.jpeg({ quality: fileSize < 50 * 1024 ? 100 : 90 })
1010
.toBuffer()
1111
);
1212
};
@@ -19,21 +19,15 @@ export const validateAndProccessFile = async (file: File) => {
1919
return null;
2020
}
2121

22+
// Buffer once and reuse: sniffing and compressing both need the bytes, and
23+
// this can be up to 5 MB
24+
const buffer = Buffer.from(await file.arrayBuffer());
25+
2226
// Actual file type validation
23-
const fileTypeRes = await fileTypeFromBuffer(
24-
Buffer.from(await file.arrayBuffer())
25-
);
26-
if (!fileTypeRes) {
27+
const fileTypeRes = await fileTypeFromBuffer(buffer);
28+
if (!fileTypeRes?.mime.startsWith('image/')) {
2729
return null;
2830
}
2931

30-
// Process the file
31-
switch (true) {
32-
case fileTypeRes.mime.startsWith('image/'): {
33-
return getCompressedImage(file);
34-
}
35-
default: {
36-
return null;
37-
}
38-
}
32+
return getCompressedImage(buffer, file.size);
3933
};

apps/web/src/app/bookmark-panel/page.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
import { ScrollArea } from '@bypass/ui';
1515
import { useVirtualizer } from '@tanstack/react-virtual';
1616
import { useSearchParams } from 'next/navigation';
17-
import { useCallback, useEffect, useRef, useState } from 'react';
17+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
1818

1919
import { getFromLocalStorage } from '@app/utils/storage';
2020

@@ -30,9 +30,9 @@ export default function BookmarksPage() {
3030
);
3131
const [folders, setFolders] = useState<IBookmarksObj['folders']>({});
3232
const [searchText, setSearchText] = useState('');
33-
const filteredContextBookmarks = getFilteredContextBookmarks(
34-
contextBookmarks,
35-
searchText
33+
const filteredContextBookmarks = useMemo(
34+
() => getFilteredContextBookmarks(contextBookmarks, searchText),
35+
[contextBookmarks, searchText]
3636
);
3737
const virtualizer = useVirtualizer({
3838
count: filteredContextBookmarks.length,

apps/web/src/app/helpers/firebase/auth.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ export const googleSignOut = async () => signOut(auth);
3838
export const onAuthStateChange = (callback: (user: User | null) => void) =>
3939
onAuthStateChanged(auth, callback);
4040

41-
export const getAuthIdToken = async () => auth.currentUser?.getIdToken(true);
41+
// No force-refresh: this runs before every tRPC request and the SDK already
42+
// refreshes within 5 mins of expiry, so forcing it added a securetoken round
43+
// trip to each call
44+
export const getAuthIdToken = async () => auth.currentUser?.getIdToken();
4245

4346
export const emailAndPasswordSignIn = async (
4447
email: string,

packages/shared/src/components/Persons/hooks/usePerson.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { useCallback } from 'react';
22

33
import { ECacheBucketKeys } from '../../../constants/cache';
44
import useStorage from '../../../hooks/useStorage';
5-
import { getBlobUrlFromCache } from '../../../utils/cache';
5+
import {
6+
getBlobUrlFromCache,
7+
getBlobUrlFromOpenCache,
8+
getCacheObj,
9+
} from '../../../utils/cache';
610
import { type IPerson, type IPersonWithImage } from '../interfaces/persons';
711
import { decodePersons } from '../utils';
812

@@ -29,17 +33,27 @@ const usePerson = () => {
2933

3034
const getPersonsWithImageUrl = useCallback(
3135
async (persons: IPerson[]): Promise<IPersonWithImage[]> => {
32-
if (!persons) {
36+
if (!persons?.length) {
3337
return [];
3438
}
39+
const personImages = await getPersonImageUrls();
40+
if (!personImages) {
41+
// Matches resolvePersonImageFromUid: bail before touching CacheStorage
42+
return persons.map((person) => ({ ...person, imageUrl: '' }));
43+
}
44+
// Open the bucket once for the whole list rather than once per person
45+
const cache = await getCacheObj(ECacheBucketKeys.person);
3546
return Promise.all(
3647
persons.map(async (person) => ({
3748
...person,
38-
imageUrl: await resolvePersonImageFromUid(person.uid),
49+
imageUrl: await getBlobUrlFromOpenCache(
50+
cache,
51+
personImages[person.uid]
52+
),
3953
}))
4054
);
4155
},
42-
[resolvePersonImageFromUid]
56+
[getPersonImageUrls]
4357
);
4458

4559
const getPersonTaggedUrls = useCallback(

packages/shared/src/components/Persons/hooks/useTaggedBookmarks.ts

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,49 @@
11
import useSWR from 'swr';
22

3-
import useBookmark from '../../Bookmarks/hooks/useBookmark';
4-
import { getDecryptedBookmark } from '../../Bookmarks/utils';
3+
import useStorage from '../../../hooks/useStorage';
4+
import { ROOT_FOLDER_ID } from '../../Bookmarks/constants';
5+
import {
6+
getDecryptedBookmark,
7+
getDecryptedFolder,
8+
getDefaultFolder,
9+
} from '../../Bookmarks/utils';
510
import { type IBookmarkWithFolder } from '../interfaces/bookmark';
611
import { getOrderedBookmarksList } from '../utils/bookmark';
7-
import usePerson from './usePerson';
812

913
const useTaggedBookmarks = (personUid = '') => {
10-
const { getBookmarkFromHash, getFolderFromHash, getDefaultOrRootFolderUrls } =
11-
useBookmark();
12-
const { getPersonTaggedUrls } = usePerson();
14+
const { getBookmarks } = useStorage();
1315

1416
return useSWR(
1517
personUid ? ['tagged-bookmarks', personUid] : null,
1618
async () => {
17-
const taggedUrls = await getPersonTaggedUrls(personUid);
18-
if (!taggedUrls?.length) {
19+
// One read of the whole bookmarks object for the entire list. Resolving
20+
// each tagged url through useBookmark/usePerson would re-read and
21+
// re-parse it twice per bookmark.
22+
const bookmarks = await getBookmarks();
23+
if (!bookmarks?.urlList) {
1924
return [];
2025
}
26+
const { urlList, folderList, folders } = bookmarks;
2127

22-
const fetchedBookmarks = await Promise.all(
23-
taggedUrls.map(async (urlHash) => {
24-
const bookmark = await getBookmarkFromHash(urlHash);
25-
const parent = await getFolderFromHash(bookmark.parentHash);
26-
const decodedBookmark = getDecryptedBookmark(bookmark);
27-
return Object.assign(decodedBookmark, {
28+
const fetchedBookmarks = Object.entries(urlList)
29+
.filter(([, bookmark]) => bookmark.taggedPersons.includes(personUid))
30+
.map(([, bookmark]) => {
31+
const parent = getDecryptedFolder(folderList[bookmark.parentHash]);
32+
return Object.assign(getDecryptedBookmark(bookmark), {
2833
parentName: parent.name,
2934
parentId: parent.id,
3035
}) satisfies IBookmarkWithFolder;
31-
})
32-
);
33-
const defaultUrls = await getDefaultOrRootFolderUrls();
36+
});
37+
if (!fetchedBookmarks.length) {
38+
return [];
39+
}
40+
41+
const parentHash =
42+
getDefaultFolder(Object.values(folderList))?.id ?? ROOT_FOLDER_ID;
43+
const defaultUrls = Object.values(folders[parentHash])
44+
.filter((bookmark) => !bookmark.isDir)
45+
.map((urlData) => urlList[urlData.hash]);
46+
3447
return getOrderedBookmarksList(fetchedBookmarks, defaultUrls);
3548
}
3649
);

packages/shared/src/utils/cache.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@ const getFromCache = async (cacheBucketKey: ECacheBucketKeys, url: string) => {
4747
return cache.match(url);
4848
};
4949

50+
/**
51+
* Takes an already-open Cache so callers resolving many urls can open the
52+
* bucket once instead of per url.
53+
*/
54+
export const getBlobUrlFromOpenCache = async (cache: Cache, url: string) => {
55+
const response = await cache.match(url);
56+
const blob = await response?.blob();
57+
if (!blob) {
58+
return '';
59+
}
60+
return URL.createObjectURL(blob);
61+
};
62+
5063
export const getBlobUrlFromCache = async (
5164
cacheBucketKey: ECacheBucketKeys,
5265
url: string

0 commit comments

Comments
 (0)