From d7162ef570f0eef7f72718ed6dd42404ef83a604 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 8 Sep 2026 19:41:15 -0700 Subject: [PATCH] perf: reuse collators when scanning Warp themes (#19441) * perf: reuse collators when scanning Warp themes * perf(warp-themes): filter before collating and skip trivial sorts Warp discovery collated every entry in the user's home or %APPDATA%\\warp before discarding the non-Warp ones; filter first so ICU only sees candidate names (order is unchanged: filtering commutes with a stable total-order sort). Also skip the collator entirely for 0/1-entry directories and single-file dialog picks, and drop the sort that ran only to be thrown away when the preview budget expired. --------- Co-authored-by: m4air Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- src/main/warp-themes/directory-entry-order.ts | 11 +++ src/main/warp-themes/discovery.test.ts | 72 +++++++++++++++++++ src/main/warp-themes/discovery.ts | 34 ++++----- .../manual-warp-theme-files.test.ts | 47 ++++++++++++ .../warp-themes/manual-warp-theme-files.ts | 41 ++++++----- .../warp-themes/theme-file-scanner.test.ts | 36 ++++++++++ src/main/warp-themes/theme-file-scanner.ts | 14 +--- 7 files changed, 205 insertions(+), 50 deletions(-) create mode 100644 src/main/warp-themes/directory-entry-order.ts create mode 100644 src/main/warp-themes/manual-warp-theme-files.test.ts create mode 100644 src/main/warp-themes/theme-file-scanner.test.ts diff --git a/src/main/warp-themes/directory-entry-order.ts b/src/main/warp-themes/directory-entry-order.ts new file mode 100644 index 00000000000..1b4cbf81bcb --- /dev/null +++ b/src/main/warp-themes/directory-entry-order.ts @@ -0,0 +1,11 @@ +// Warp theme discovery walks user home and app-data trees, so callers filter to +// the entries they want *before* sorting: same resulting order (the comparator is +// a total order over a stable sort), without collating the hundreds of unrelated +// names a home directory holds. +export function sortDirectoryEntriesByName(entries: T[]): T[] { + if (entries.length < 2) { + return entries + } + const compare = new Intl.Collator(undefined, { sensitivity: 'base' }).compare + return entries.sort((left, right) => compare(left.name, right.name)) +} diff --git a/src/main/warp-themes/discovery.test.ts b/src/main/warp-themes/discovery.test.ts index 977a5b30de7..0a6851019c4 100644 --- a/src/main/warp-themes/discovery.test.ts +++ b/src/main/warp-themes/discovery.test.ts @@ -43,6 +43,78 @@ describe('getWarpThemeDirectories', () => { readdirSyncMock.mockReturnValue([]) }) + it('sorts dynamic directories with one collator and preserves locale ties', () => { + platformMock.mockReturnValue('darwin') + const names = ['éclair', 'Eclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul'].map( + (name) => `.warp-${name}` + ) + readdirSyncMock.mockReturnValue(names.map(directoryEntry)) + const expected = [...names].sort((a, b) => + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle. + a.localeCompare(b, undefined, { sensitivity: 'base' }) + ) + const NativeCollator = Intl.Collator + const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) { + return new NativeCollator(locales, options) + }) + const localeCompare = vi.spyOn(String.prototype, 'localeCompare') + try { + expect(getWarpThemeDirectories().slice(6)).toEqual( + expected.map((name) => `/Users/alice/${name}/themes`) + ) + expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' }) + expect(localeCompare).not.toHaveBeenCalled() + } finally { + construct.mockRestore() + localeCompare.mockRestore() + } + }) + + it('collates only the Warp entries in a crowded home directory', () => { + platformMock.mockReturnValue('darwin') + const noise = Array.from({ length: 500 }, (_, index) => directoryEntry(`project-${index}`)) + const warpNames = ['.warp-zebra', '.warp-Ångström', '.warp-éclair'] + readdirSyncMock.mockReturnValue([...noise, ...warpNames.map(directoryEntry)]) + const expected = [...warpNames].sort((a, b) => + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle. + a.localeCompare(b, undefined, { sensitivity: 'base' }) + ) + const NativeCollator = Intl.Collator + const compares: string[][] = [] + vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) { + const collator = new NativeCollator(locales, options) + return { + ...collator, + compare: (left: string, right: string) => { + compares.push([left, right]) + return collator.compare(left, right) + } + } + }) + try { + expect(getWarpThemeDirectories().slice(6)).toEqual( + expected.map((name) => `/Users/alice/${name}/themes`) + ) + // Filtering first keeps the crowd out of ICU: only Warp names are collated. + expect(compares.flat().every((name) => name.startsWith('.warp'))).toBe(true) + expect(compares.length).toBeLessThan(10) + } finally { + vi.restoreAllMocks() + } + }) + + it('skips a directory scan that finds no dynamic Warp entries', () => { + platformMock.mockReturnValue('darwin') + readdirSyncMock.mockReturnValue([directoryEntry('Documents'), fileEntry('.warprc')]) + const construct = vi.spyOn(Intl, 'Collator') + try { + expect(getWarpThemeDirectories()).toHaveLength(6) + expect(construct).not.toHaveBeenCalled() + } finally { + vi.restoreAllMocks() + } + }) + it('returns macOS Warp channel theme directories in stable-first order', () => { platformMock.mockReturnValue('darwin') expect(getWarpThemeDirectories()).toEqual([ diff --git a/src/main/warp-themes/discovery.ts b/src/main/warp-themes/discovery.ts index fd29419c12c..68f8a70c654 100644 --- a/src/main/warp-themes/discovery.ts +++ b/src/main/warp-themes/discovery.ts @@ -2,6 +2,7 @@ import { readdirSync } from 'node:fs' import type { Dirent } from 'node:fs' import { homedir, platform } from 'node:os' import path from 'node:path' +import { sortDirectoryEntriesByName } from './directory-entry-order' const WARP_CHANNELS = [ { macName: '.warp', linuxName: 'warp-terminal', windowsName: 'Warp' }, @@ -16,10 +17,13 @@ const WARP_CHANNELS = [ } ] -function readDirectoryEntries(directoryPath: string): Dirent[] { +function readDirectoryEntries( + directoryPath: string, + include: (entry: Dirent) => boolean +): Dirent[] { try { - return readdirSync(directoryPath, { withFileTypes: true }).sort((left, right) => - left.name.localeCompare(right.name, undefined, { sensitivity: 'base' }) + return sortDirectoryEntriesByName( + readdirSync(directoryPath, { withFileTypes: true }).filter(include) ) } catch { return [] @@ -57,9 +61,10 @@ function getMacWarpThemeDirectories(home: string): string[] { return warpThemeDirectoriesFromDataHomes( [ ...WARP_CHANNELS.map((channel) => pathImpl.join(home, channel.macName)), - ...readDirectoryEntries(home) - .filter((entry) => entry.isDirectory() && entry.name.startsWith('.warp')) - .map((entry) => pathImpl.join(home, entry.name)) + ...readDirectoryEntries( + home, + (entry) => entry.isDirectory() && entry.name.startsWith('.warp') + ).map((entry) => pathImpl.join(home, entry.name)) ], pathImpl ) @@ -77,13 +82,11 @@ function getLinuxWarpThemeDirectories(home: string): string[] { return warpThemeDirectoriesFromDataHomes( [ ...WARP_CHANNELS.map((channel) => pathImpl.join(dataHome, channel.linuxName)), - ...readDirectoryEntries(dataHome) - .filter( - (entry) => - entry.isDirectory() && - (entry.name === 'warp-terminal' || entry.name.startsWith('warp-')) - ) - .map((entry) => pathImpl.join(dataHome, entry.name)) + ...readDirectoryEntries( + dataHome, + (entry) => + entry.isDirectory() && (entry.name === 'warp-terminal' || entry.name.startsWith('warp-')) + ).map((entry) => pathImpl.join(dataHome, entry.name)) ], pathImpl ) @@ -102,10 +105,7 @@ function getWindowsWarpThemeDirectories(home: string): string[] { path.win32 ) } - for (const entry of readDirectoryEntries(warpAppData)) { - if (!entry.isDirectory()) { - continue - } + for (const entry of readDirectoryEntries(warpAppData, (entry) => entry.isDirectory())) { addDedupeDirectory( directories, seenDirectories, diff --git a/src/main/warp-themes/manual-warp-theme-files.test.ts b/src/main/warp-themes/manual-warp-theme-files.test.ts new file mode 100644 index 00000000000..975e40414dc --- /dev/null +++ b/src/main/warp-themes/manual-warp-theme-files.test.ts @@ -0,0 +1,47 @@ +import path from 'node:path' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ BrowserWindow: {}, dialog: {} })) + +import { createManualWarpThemeFileCandidates } from './manual-warp-theme-files' + +describe('manual theme ordering', () => { + it('reuses one collator for labels and path ties without changing the selected order', () => { + const names = ['éclair', 'Eclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul'] + const paths = Array.from({ length: 200 }, (_, index) => + path.join('themes', names[index % names.length]!, `${names[(index * 3) % names.length]}.yaml`) + ) + const expected = [...paths].sort( + (a, b) => + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle. + path.basename(a).localeCompare(path.basename(b), undefined, { sensitivity: 'base' }) || + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle. + a.localeCompare(b, undefined, { sensitivity: 'base' }) + ) + const NativeCollator = Intl.Collator + const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) { + return new NativeCollator(locales, options) + }) + const localeCompare = vi.spyOn(String.prototype, 'localeCompare') + try { + expect(createManualWarpThemeFileCandidates(paths).map((file) => file.path)).toEqual(expected) + expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' }) + expect(localeCompare).not.toHaveBeenCalled() + } finally { + vi.restoreAllMocks() + } + }) + + it('returns a single dialog selection without collating', () => { + const construct = vi.spyOn(Intl, 'Collator') + try { + expect(createManualWarpThemeFileCandidates([]).map((file) => file.path)).toEqual([]) + expect(createManualWarpThemeFileCandidates(['a/one.yaml']).map((file) => file.path)).toEqual([ + 'a/one.yaml' + ]) + expect(construct).not.toHaveBeenCalled() + } finally { + vi.restoreAllMocks() + } + }) +}) diff --git a/src/main/warp-themes/manual-warp-theme-files.ts b/src/main/warp-themes/manual-warp-theme-files.ts index 496ad9e3919..b50ff8ee48b 100644 --- a/src/main/warp-themes/manual-warp-theme-files.ts +++ b/src/main/warp-themes/manual-warp-theme-files.ts @@ -2,29 +2,28 @@ import { createHash } from 'node:crypto' import path from 'node:path' import { BrowserWindow, dialog, type OpenDialogOptions, type WebContents } from 'electron' import type { WarpThemeImportSkippedFile } from '../../shared/terminal-custom-themes' -import { - compareThemeFileLabels, - isYamlFile, - MAX_THEME_FILES, - type ThemeFileCandidate -} from './theme-file-scanner' +import { isYamlFile, MAX_THEME_FILES, type ThemeFileCandidate } from './theme-file-scanner' export function createManualWarpThemeFileCandidates(filePaths: string[]): ThemeFileCandidate[] { - return filePaths - .map((filePath) => ({ - path: filePath, - label: path.basename(filePath), - contentHashDiscriminator: true - })) - .sort((left, right) => { - const labelComparison = compareThemeFileLabels(left, right) - if (labelComparison !== 0) { - return labelComparison - } - // Why: manual dialogs can return selections in click order. Sort only in - // main so duplicate basenames get deterministic IDs without persisting paths. - return left.path.localeCompare(right.path, undefined, { sensitivity: 'base' }) - }) + const candidates = filePaths.map((filePath) => ({ + path: filePath, + label: path.basename(filePath), + contentHashDiscriminator: true + })) + // Picking a single file is the common dialog outcome and needs no collation. + if (candidates.length < 2) { + return candidates + } + const compareLabels = new Intl.Collator(undefined, { sensitivity: 'base' }).compare + return candidates.sort((left, right) => { + const labelComparison = compareLabels(left.label, right.label) + if (labelComparison !== 0) { + return labelComparison + } + // Why: manual dialogs can return selections in click order. Sort only in + // main so duplicate basenames get deterministic IDs without persisting paths. + return compareLabels(left.path, right.path) + }) } export function manualWarpThemeContentDiscriminator(label: string, content: string): string { diff --git a/src/main/warp-themes/theme-file-scanner.test.ts b/src/main/warp-themes/theme-file-scanner.test.ts new file mode 100644 index 00000000000..bbfd5639e65 --- /dev/null +++ b/src/main/warp-themes/theme-file-scanner.test.ts @@ -0,0 +1,36 @@ +import { mkdir, mkdtemp, readdir, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { expect, it, vi } from 'vitest' +import { scanWarpThemeDirectory } from './theme-file-scanner' + +it('reuses one collator per directory while preserving capped scan order', async () => { + const directory = await mkdtemp(path.join(tmpdir(), 'orca-theme-order-')) + const names = ['éclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul'] + try { + await Promise.all(names.map((name) => writeFile(path.join(directory, `${name}.yaml`), ''))) + await mkdir(path.join(directory, 'nested')) + await writeFile(path.join(directory, 'nested', 'theme.yaml'), '') + const expected = (await readdir(directory)) + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle. + .sort((a, b) => a.localeCompare(b, undefined, { sensitivity: 'base' })) + .map((name) => (name === 'nested' ? path.join(name, 'theme.yaml') : name)) + const NativeCollator = Intl.Collator + const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) { + return new NativeCollator(locales, options) + }) + const localeCompare = vi.spyOn(String.prototype, 'localeCompare') + try { + const result = await scanWarpThemeDirectory(directory, undefined, { themeFileLimit: 6 }) + expect(result.files.map((file) => file.label)).toEqual(expected.slice(0, 6)) + expect(result.themeFileLimitHit).toBe(true) + // One directory needs collation; the single-entry nested folder needs none. + expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' }) + expect(localeCompare).not.toHaveBeenCalled() + } finally { + vi.restoreAllMocks() + } + } finally { + await rm(directory, { recursive: true, force: true }) + } +}) diff --git a/src/main/warp-themes/theme-file-scanner.ts b/src/main/warp-themes/theme-file-scanner.ts index ba20abe0cc2..2602acb71da 100644 --- a/src/main/warp-themes/theme-file-scanner.ts +++ b/src/main/warp-themes/theme-file-scanner.ts @@ -2,6 +2,7 @@ import { opendir } from 'node:fs/promises' import type { Dirent } from 'node:fs' import path from 'node:path' import type { WarpThemeImportSkippedFile } from '../../shared/terminal-custom-themes' +import { sortDirectoryEntriesByName } from './directory-entry-order' export const MAX_THEME_FILES = 200 const MAX_THEME_DIRECTORY_DEPTH = 3 @@ -44,17 +45,6 @@ export function isYamlFile(filePath: string): boolean { return YAML_EXTENSIONS.has(path.extname(filePath).toLowerCase()) } -export function compareThemeFileLabels( - left: ThemeFileCandidate, - right: ThemeFileCandidate -): number { - return left.label.localeCompare(right.label, undefined, { sensitivity: 'base' }) -} - -function compareDirentNames(left: Dirent, right: Dirent): number { - return left.name.localeCompare(right.name, undefined, { sensitivity: 'base' }) -} - function isYamlFileEntry(entry: Dirent): boolean { return (entry.isFile() || entry.isSymbolicLink()) && isYamlFile(entry.name) } @@ -141,10 +131,10 @@ async function collectYamlFilesFromDirectory( return } - const sortedEntries = entries.sort(compareDirentNames) if (previewBudgetExpiredWhileReading) { return } + const sortedEntries = sortDirectoryEntriesByName(entries) if (entryLimitHit && !budget.entryLimitReported) { skippedFiles.push({ label: relativeDirectory || sourceLabel,