mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
perf(images): key icon-validation memo on the persisted icon object
Replaces the per-source 64-entry FIFO string-key memo with a WeakMap keyed on
the persisted repoIcon object that hydrateRepo already receives, storing
{src, source, supported} and re-checking both fields on a hit.
Retention becomes zero by construction (entries die with state.repos[i].repoIcon),
so there is no cap to evict live icons and no dead icon strings held after a repo
or icon is replaced. The identity re-check makes an in-place mutation unable to
serve a stale verdict. Drops bounded-string-key-memo.ts and reverts the collateral
terminal-title-classification-memo refactor.
This commit is contained in:
@@ -1,29 +0,0 @@
|
||||
/**
|
||||
* FIFO-bounded memo for a pure `(key: string) => T`.
|
||||
*
|
||||
* Insertion-ordered eviction: the oldest key is the one whose input has been superseded longest, so
|
||||
* it is the least likely to be asked for again. Entries hold a reference to a string the caller
|
||||
* already retains, so a live entry costs the map slot and nothing else.
|
||||
*/
|
||||
export function memoizeByStringKey<T>(
|
||||
compute: (key: string) => T,
|
||||
maxEntries: number
|
||||
): (key: string) => T {
|
||||
// Boxed values so `undefined`/`null` results are still cache hits.
|
||||
const cache = new Map<string, { value: T }>()
|
||||
return (key: string): T => {
|
||||
const cached = cache.get(key)
|
||||
if (cached) {
|
||||
return cached.value
|
||||
}
|
||||
const value = compute(key)
|
||||
if (cache.size >= maxEntries) {
|
||||
const oldest = cache.keys().next()
|
||||
if (!oldest.done) {
|
||||
cache.delete(oldest.value)
|
||||
}
|
||||
}
|
||||
cache.set(key, { value })
|
||||
return value
|
||||
}
|
||||
}
|
||||
@@ -207,7 +207,6 @@ describe('githubAvatarSlug', () => {
|
||||
|
||||
describe('repo icon source validation memo', () => {
|
||||
const HYDRATIONS = 25
|
||||
const MAX_MEMOIZED_ICON_SOURCES = 64
|
||||
|
||||
function uploadIcon(width: number): { type: 'image'; src: string; source: 'upload' } {
|
||||
return { type: 'image', src: `data:image/png;base64,${pngBase64(width, 1)}`, source: 'upload' }
|
||||
@@ -250,15 +249,39 @@ describe('repo icon source validation memo', () => {
|
||||
expect(sanitizeRepoIcon({ type: 'image', src, source: 'upload' })).toBeUndefined()
|
||||
})
|
||||
|
||||
it('evicts oldest entries instead of growing without bound', () => {
|
||||
const overflow = MAX_MEMOIZED_ICON_SOURCES + 6
|
||||
for (let index = 1; index <= overflow; index += 1) {
|
||||
sanitizeRepoIcon(uploadIcon(1000 + index))
|
||||
// Guard for the removed cap: the memo hangs off the persisted icon object, so it holds a verdict
|
||||
// for every live icon no matter how many there are. A fixed-size map would evict the earliest
|
||||
// entries here and re-decode them on the next hydration.
|
||||
it('keeps a verdict for every live icon, however many repos have one', () => {
|
||||
const LIVE_ICONS = 200
|
||||
const icons = Array.from({ length: LIVE_ICONS }, (_, index) => uploadIcon(1000 + index))
|
||||
for (const icon of icons) {
|
||||
sanitizeRepoIcon(icon)
|
||||
}
|
||||
dataUriValidations.count = 0
|
||||
sanitizeRepoIcon(uploadIcon(1000 + overflow))
|
||||
|
||||
for (const icon of icons) {
|
||||
expect(sanitizeRepoIcon(icon)).toEqual(icon)
|
||||
}
|
||||
|
||||
expect(dataUriValidations.count).toBe(0)
|
||||
sanitizeRepoIcon(uploadIcon(1001))
|
||||
})
|
||||
|
||||
// Guard for the hazard object keying introduces: the stored src/source are re-checked on a hit,
|
||||
// so a persisted icon edited in place can never be served its previous verdict.
|
||||
it('re-validates an icon object whose src or source is mutated in place', () => {
|
||||
const icon = { type: 'image', src: `data:image/png;base64,${pngBase64(7, 1)}`, source: 'file' }
|
||||
expect(sanitizeRepoIcon(icon)).toEqual(icon)
|
||||
|
||||
icon.src = `data:image/png;base64,${pngBase64(8, 1)}`
|
||||
dataUriValidations.count = 0
|
||||
expect(sanitizeRepoIcon(icon)).toEqual(icon)
|
||||
expect(dataUriValidations.count).toBe(1)
|
||||
|
||||
// WebP is a `file` icon but not an `upload` icon, so the same object must flip verdicts.
|
||||
icon.src = `data:image/webp;base64,${WEBP_1X1_BASE64}`
|
||||
expect(sanitizeRepoIcon(icon)).toEqual(icon)
|
||||
icon.source = 'upload'
|
||||
expect(sanitizeRepoIcon(icon)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
+24
-33
@@ -1,4 +1,3 @@
|
||||
import { memoizeByStringKey } from './bounded-string-key-memo'
|
||||
import { validateRasterImageDataUri } from './image-data-uri'
|
||||
|
||||
export type RepoIconImageSource = 'upload' | 'file' | 'favicon' | 'github'
|
||||
@@ -80,13 +79,6 @@ function normalizeGitHubAvatarHost(rawHost?: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cap: comfortably above the icon working set of a heavy multi-repo user, small enough that a
|
||||
* session of icon edits cannot grow the map without limit. Keys are the same `src` strings the
|
||||
* persisted repo list already holds, so a live entry costs the map slot and nothing else.
|
||||
*/
|
||||
const MAX_MEMOIZED_ICON_SOURCES = 64
|
||||
|
||||
function computeIsSupportedImageSrc(src: string, source: RepoIconImageSource): boolean {
|
||||
if (source === 'upload') {
|
||||
return (
|
||||
@@ -120,33 +112,32 @@ function computeIsSupportedImageSrc(src: string, source: RepoIconImageSource): b
|
||||
return url.hostname === 'www.google.com' && url.pathname === '/s2/favicons'
|
||||
}
|
||||
|
||||
type ImageSrcVerdict = { src: unknown; source: unknown; supported: boolean }
|
||||
|
||||
/**
|
||||
* Why: repo icons are immutable persisted strings, so a given `src` always validates the same way
|
||||
* and there is no invalidation window — a changed icon is simply a new key. `getRepos()` re-hydrates
|
||||
* every repo on every call, and validating one inline data URI means scanning it twice with a regex
|
||||
* and base64-decoding its header.
|
||||
* Why: `getRepos()` re-hydrates every repo on every call, and validating one inline data URI means
|
||||
* scanning a 400 KB string twice with a regex and base64-decoding its header. `hydrateRepo` is
|
||||
* handed the *same* persisted `repoIcon` object every time, so the verdict is cached on that object
|
||||
* and dies with it — no cap, no eviction, and nothing retained once a repo or an icon is replaced.
|
||||
*
|
||||
* One memo per source rather than a composite key, because V8 caches a string's hash in the string
|
||||
* itself: keying on `src` makes a 400 KB data URI an O(1) lookup, while `${source}\0${src}` would
|
||||
* rebuild and rehash the whole thing on every call.
|
||||
* `src`/`source` are re-checked on a hit, so mutating the persisted icon in place cannot serve a
|
||||
* stale verdict. Both are the identical string references in the steady state, so the compare is a
|
||||
* pointer check, not a 400 KB scan.
|
||||
*/
|
||||
const memoizedIsSupportedImageSrc: Record<RepoIconImageSource, (src: string) => boolean> = {
|
||||
upload: memoizeByStringKey(
|
||||
(src) => computeIsSupportedImageSrc(src, 'upload'),
|
||||
MAX_MEMOIZED_ICON_SOURCES
|
||||
),
|
||||
file: memoizeByStringKey(
|
||||
(src) => computeIsSupportedImageSrc(src, 'file'),
|
||||
MAX_MEMOIZED_ICON_SOURCES
|
||||
),
|
||||
favicon: memoizeByStringKey(
|
||||
(src) => computeIsSupportedImageSrc(src, 'favicon'),
|
||||
MAX_MEMOIZED_ICON_SOURCES
|
||||
),
|
||||
github: memoizeByStringKey(
|
||||
(src) => computeIsSupportedImageSrc(src, 'github'),
|
||||
MAX_MEMOIZED_ICON_SOURCES
|
||||
)
|
||||
const imageSrcVerdicts = new WeakMap<object, ImageSrcVerdict>()
|
||||
|
||||
function isSupportedImageSrc(
|
||||
candidate: Record<string, unknown>,
|
||||
src: string,
|
||||
source: RepoIconImageSource
|
||||
): boolean {
|
||||
const cached = imageSrcVerdicts.get(candidate)
|
||||
if (cached && cached.src === candidate.src && cached.source === candidate.source) {
|
||||
return cached.supported
|
||||
}
|
||||
const supported = computeIsSupportedImageSrc(src, source)
|
||||
imageSrcVerdicts.set(candidate, { src: candidate.src, source: candidate.source, supported })
|
||||
return supported
|
||||
}
|
||||
|
||||
export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined {
|
||||
@@ -183,7 +174,7 @@ export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined {
|
||||
if (!isRepoIconImageSource(source) || src.length > MAX_REPO_ICON_DATA_URL_LENGTH) {
|
||||
return undefined
|
||||
}
|
||||
if (!memoizedIsSupportedImageSrc[source](src)) {
|
||||
if (!isSupportedImageSrc(candidate, src, source)) {
|
||||
return undefined
|
||||
}
|
||||
const label = typeof candidate.label === 'string' ? candidate.label.trim().slice(0, 80) : ''
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
import { memoizeByStringKey } from './bounded-string-key-memo'
|
||||
|
||||
/**
|
||||
* Bounded memo for pure `(title: string) => T` terminal-title classifiers.
|
||||
*
|
||||
@@ -23,5 +21,23 @@ const MAX_MEMOIZED_TITLES = 1024
|
||||
export function memoizeTitleClassification<T>(
|
||||
classify: (title: string) => T
|
||||
): (title: string) => T {
|
||||
return memoizeByStringKey(classify, MAX_MEMOIZED_TITLES)
|
||||
// Boxed values so `undefined`/`null` verdicts are still cache hits.
|
||||
const cache = new Map<string, { value: T }>()
|
||||
return (title: string): T => {
|
||||
const cached = cache.get(title)
|
||||
if (cached) {
|
||||
return cached.value
|
||||
}
|
||||
const value = classify(title)
|
||||
// Insertion-ordered FIFO eviction: a pane's superseded title frames are the
|
||||
// oldest keys and the least likely to be asked for again.
|
||||
if (cache.size >= MAX_MEMOIZED_TITLES) {
|
||||
const oldest = cache.keys().next()
|
||||
if (!oldest.done) {
|
||||
cache.delete(oldest.value)
|
||||
}
|
||||
}
|
||||
cache.set(title, { value })
|
||||
return value
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user