diff --git a/changes/dictionary-freeze-and-appimage-notifications.md b/changes/dictionary-freeze-and-appimage-notifications.md new file mode 100644 index 00000000..5bc6e4c9 --- /dev/null +++ b/changes/dictionary-freeze-and-appimage-notifications.md @@ -0,0 +1,5 @@ +type: fixed +area: dictionary + +- Character dictionary generation, merged rebuilds, and imports no longer freeze the app (and trigger the compositor's "application not responding" dialog) on large dictionaries; snapshot reads/writes, archive building, and the character image/name lookup caches now do their heavy work off the UI's critical path. +- Desktop progress notifications now update in place on Linux AppImage installs too: the AppImage's bundled libraries broke the system notify-send helper, which silently forced the flickering close-and-reopen notification fallback. diff --git a/src/main/character-dictionary-runtime.ts b/src/main/character-dictionary-runtime.ts index 56b71ecb..c362d80b 100644 --- a/src/main/character-dictionary-runtime.ts +++ b/src/main/character-dictionary-runtime.ts @@ -244,13 +244,13 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar }; }; - const findCachedSnapshotForSeriesKey = ( + const findCachedSnapshotForSeriesKey = async ( seriesKey: string, fallbackSeriesKey?: string, - ): CharacterDictionarySnapshot | null => { + ): Promise => { const acceptedKeys = new Set([seriesKey, fallbackSeriesKey].filter(Boolean)); return ( - readCachedSnapshots(outputDir).find((snapshot) => { + (await readCachedSnapshots(outputDir)).find((snapshot) => { const snapshotSeriesKey = buildCharacterDictionarySeriesKey({ mediaPath: null, mediaTitle: snapshot.mediaTitle, @@ -293,7 +293,9 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar const cachedResolution = readCachedMediaResolution(outputDir, seriesKey); if (cachedResolution) { - const cachedSnapshot = readSnapshot(getSnapshotPath(outputDir, cachedResolution.mediaId)); + const cachedSnapshot = await readSnapshot( + getSnapshotPath(outputDir, cachedResolution.mediaId), + ); if (cachedSnapshot) { deps.logInfo?.( `[dictionary] cached AniList match: ${cachedSnapshot.mediaTitle} -> AniList ${cachedSnapshot.mediaId}`, @@ -305,7 +307,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar } } - const cachedSnapshot = findCachedSnapshotForSeriesKey(seriesKey, unscopedSeriesKey); + const cachedSnapshot = await findCachedSnapshotForSeriesKey(seriesKey, unscopedSeriesKey); if (cachedSnapshot) { writeCachedMediaResolution(outputDir, { seriesKey, @@ -348,7 +350,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar progress?: CharacterDictionarySnapshotProgressCallbacks, ): Promise => { const snapshotPath = getSnapshotPath(outputDir, mediaId); - const cachedSnapshot = readSnapshot(snapshotPath); + const cachedSnapshot = await readSnapshot(snapshotPath); const refreshReason = cachedSnapshot ? getCachedSnapshotRefreshReason(cachedSnapshot) : null; if (cachedSnapshot && refreshReason === null) { deps.logInfo?.(`[dictionary] snapshot hit for AniList ${mediaId}`); @@ -485,7 +487,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar resolvedNameSplits, nameSplitSource, ); - writeSnapshot(snapshotPath, snapshot); + await writeSnapshot(snapshotPath, snapshot); deps.logInfo?.( `[dictionary] stored snapshot for AniList ${mediaId}: ${snapshot.entryCount} terms`, ); @@ -526,19 +528,22 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar const snapshotResults = await Promise.all( normalizedMediaIds.map((mediaId) => getOrCreateSnapshot(mediaId)), ); - const snapshots = snapshotResults.map(({ mediaId }) => { - const snapshot = readSnapshot(getSnapshotPath(outputDir, mediaId)); + // Sequential on purpose: each snapshot parse is a chunk of main-thread work, so reading them + // one at a time keeps the event loop breathing between files. + const snapshots: CharacterDictionarySnapshot[] = []; + for (const { mediaId } of snapshotResults) { + const snapshot = await readSnapshot(getSnapshotPath(outputDir, mediaId)); if (!snapshot) { throw new Error(`Missing character dictionary snapshot for AniList ${mediaId}.`); } - return snapshot; - }); + snapshots.push(snapshot); + } const revision = buildMergedRevision(normalizedMediaIds, snapshots); const description = snapshots.length === 1 ? `Character names from ${snapshots[0]!.mediaTitle}` : `Character names from ${snapshots.length} recent anime`; - const { zipPath, entryCount } = buildDictionaryZip( + const { zipPath, entryCount } = await buildDictionaryZip( getMergedZipPath(outputDir), CHARACTER_DICTIONARY_MERGED_TITLE, description, @@ -633,7 +638,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar resolvedMedia.title, waitForAniListRequestSlot, ); - const storedSnapshot = readSnapshot(getSnapshotPath(outputDir, resolvedMedia.id)); + const storedSnapshot = await readSnapshot(getSnapshotPath(outputDir, resolvedMedia.id)); if (!storedSnapshot) { throw new Error(`Snapshot missing after generation for AniList ${resolvedMedia.id}.`); } @@ -642,7 +647,7 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar const description = `Character names from ${storedSnapshot.mediaTitle} [AniList media ID ${resolvedMedia.id}]`; const zipPath = path.join(outputDir, `anilist-${resolvedMedia.id}.zip`); deps.logInfo?.(`[dictionary] building ZIP for AniList ${resolvedMedia.id}`); - buildDictionaryZip( + await buildDictionaryZip( zipPath, dictionaryTitle, description, diff --git a/src/main/character-dictionary-runtime/cache.test.ts b/src/main/character-dictionary-runtime/cache.test.ts index 1d5f77b2..6014f330 100644 --- a/src/main/character-dictionary-runtime/cache.test.ts +++ b/src/main/character-dictionary-runtime/cache.test.ts @@ -29,17 +29,17 @@ function createSnapshot(): CharacterDictionarySnapshot { }; } -test('writeSnapshot persists and readSnapshot restores current-format snapshots', () => { +test('writeSnapshot persists and readSnapshot restores current-format snapshots', async () => { const outputDir = makeTempDir(); const snapshotPath = getSnapshotPath(outputDir, 130298); const snapshot = createSnapshot(); - writeSnapshot(snapshotPath, snapshot); + await writeSnapshot(snapshotPath, snapshot); - assert.deepEqual(readSnapshot(snapshotPath), { ...snapshot, nameSplitSource: 'heuristic' }); + assert.deepEqual(await readSnapshot(snapshotPath), { ...snapshot, nameSplitSource: 'heuristic' }); }); -test('readSnapshot preserves the mecab name-split source and defaults missing values to heuristic', () => { +test('readSnapshot preserves the mecab name-split source and defaults missing values to heuristic', async () => { const outputDir = makeTempDir(); const snapshotPath = getSnapshotPath(outputDir, 130298); const snapshot: CharacterDictionarySnapshot = { @@ -47,12 +47,12 @@ test('readSnapshot preserves the mecab name-split source and defaults missing va nameSplitSource: 'mecab', }; - writeSnapshot(snapshotPath, snapshot); + await writeSnapshot(snapshotPath, snapshot); - assert.equal(readSnapshot(snapshotPath)?.nameSplitSource, 'mecab'); + assert.equal((await readSnapshot(snapshotPath))?.nameSplitSource, 'mecab'); }); -test('readSnapshot ignores snapshots written with an older format version', () => { +test('readSnapshot ignores snapshots written with an older format version', async () => { const outputDir = makeTempDir(); const snapshotPath = getSnapshotPath(outputDir, 130298); const staleSnapshot = { @@ -63,10 +63,10 @@ test('readSnapshot ignores snapshots written with an older format version', () = fs.mkdirSync(path.dirname(snapshotPath), { recursive: true }); fs.writeFileSync(snapshotPath, JSON.stringify(staleSnapshot), 'utf8'); - assert.equal(readSnapshot(snapshotPath), null); + assert.equal(await readSnapshot(snapshotPath), null); }); -test('readSnapshot ignores v15 snapshots with stale romanized character-name entries', () => { +test('readSnapshot ignores v15 snapshots with stale romanized character-name entries', async () => { const outputDir = makeTempDir(); const snapshotPath = getSnapshotPath(outputDir, 130298); const staleSnapshot = { @@ -78,5 +78,5 @@ test('readSnapshot ignores v15 snapshots with stale romanized character-name ent fs.mkdirSync(path.dirname(snapshotPath), { recursive: true }); fs.writeFileSync(snapshotPath, JSON.stringify(staleSnapshot), 'utf8'); - assert.equal(readSnapshot(snapshotPath), null); + assert.equal(await readSnapshot(snapshotPath), null); }); diff --git a/src/main/character-dictionary-runtime/cache.ts b/src/main/character-dictionary-runtime/cache.ts index 2e547897..be8ecd18 100644 --- a/src/main/character-dictionary-runtime/cache.ts +++ b/src/main/character-dictionary-runtime/cache.ts @@ -102,24 +102,42 @@ export function writeCachedMediaResolution( writeMediaResolutionEntries(outputDir, [...remaining, normalized]); } -export function readCachedSnapshots(outputDir: string): CharacterDictionarySnapshot[] { +/** + * Snapshots for long series run to hundreds of MB each, so everything here reads them off the main + * thread's critical path: file IO is async and only the unavoidable JSON.parse runs on the loop, + * one file at a time. Reading the whole directory synchronously used to block the process for + * multiple seconds, long enough for the compositor to declare the app unresponsive mid-playback. + */ +export async function readCachedSnapshots( + outputDir: string, +): Promise { let entries: fs.Dirent[] = []; try { - entries = fs.readdirSync(getSnapshotsDir(outputDir), { withFileTypes: true }); + entries = await fs.promises.readdir(getSnapshotsDir(outputDir), { withFileTypes: true }); } catch { return []; } - return entries + const names = entries .filter((entry) => entry.isFile() && /^anilist-\d+\.json$/.test(entry.name)) - .sort((left, right) => left.name.localeCompare(right.name)) - .map((entry) => readSnapshot(path.join(getSnapshotsDir(outputDir), entry.name))) - .filter((snapshot): snapshot is CharacterDictionarySnapshot => snapshot !== null); + .map((entry) => entry.name) + .sort((left, right) => left.localeCompare(right)); + + const snapshots: CharacterDictionarySnapshot[] = []; + for (const name of names) { + const snapshot = await readSnapshot(path.join(getSnapshotsDir(outputDir), name)); + if (snapshot) { + snapshots.push(snapshot); + } + } + return snapshots; } -export function readSnapshot(snapshotPath: string): CharacterDictionarySnapshot | null { +export async function readSnapshot( + snapshotPath: string, +): Promise { try { - const raw = fs.readFileSync(snapshotPath, 'utf8'); + const raw = await fs.promises.readFile(snapshotPath, 'utf8'); const parsed = JSON.parse(raw) as Partial; if (!parsed || typeof parsed !== 'object') { return null; @@ -150,9 +168,59 @@ export function readSnapshot(snapshotPath: string): CharacterDictionarySnapshot } } -export function writeSnapshot(snapshotPath: string, snapshot: CharacterDictionarySnapshot): void { +// Flushing in a few-MB batches keeps each stringify-and-write slice short; a single +// JSON.stringify of a large snapshot blocks the event loop for seconds. +const SNAPSHOT_WRITE_FLUSH_BYTES = 4 * 1024 * 1024; + +/** + * Streams the snapshot to disk piece by piece instead of stringifying it in one shot, then renames + * the finished file into place so a crash mid-write (or two concurrent writers for the same media) + * can never leave a torn file where a snapshot used to be. + */ +export async function writeSnapshot( + snapshotPath: string, + snapshot: CharacterDictionarySnapshot, +): Promise { ensureDir(path.dirname(snapshotPath)); - fs.writeFileSync(snapshotPath, JSON.stringify(snapshot, null, 2), 'utf8'); + const tempPath = `${snapshotPath}.tmp-${process.pid}`; + const handle = await fs.promises.open(tempPath, 'w'); + try { + let buffered: string[] = []; + let bufferedBytes = 0; + const push = async (chunk: string): Promise => { + buffered.push(chunk); + bufferedBytes += chunk.length; + if (bufferedBytes >= SNAPSHOT_WRITE_FLUSH_BYTES) { + const joined = buffered.join(''); + buffered = []; + bufferedBytes = 0; + await handle.write(joined, null, 'utf8'); + } + }; + const writeArray = async (key: string, items: readonly unknown[]): Promise => { + await push(`,${JSON.stringify(key)}:[`); + for (let i = 0; i < items.length; i += 1) { + await push(`${i > 0 ? ',' : ''}${JSON.stringify(items[i])}`); + } + await push(']'); + }; + + const { termEntries, images, ...scalars } = snapshot; + const head = JSON.stringify(scalars); + await push(head.slice(0, -1)); + await writeArray('termEntries', termEntries); + await writeArray('images', images); + await push('}'); + if (buffered.length > 0) { + await handle.write(buffered.join(''), null, 'utf8'); + } + } catch (error) { + await handle.close(); + await fs.promises.rm(tempPath, { force: true }); + throw error; + } + await handle.close(); + await fs.promises.rename(tempPath, snapshotPath); } export function buildMergedRevision( diff --git a/src/main/character-dictionary-runtime/image-lookup.test.ts b/src/main/character-dictionary-runtime/image-lookup.test.ts index 83049f2e..aadaf39a 100644 --- a/src/main/character-dictionary-runtime/image-lookup.test.ts +++ b/src/main/character-dictionary-runtime/image-lookup.test.ts @@ -11,6 +11,22 @@ import { } from './image-lookup'; import type { CharacterDictionarySnapshot } from './types'; +// Lookup indexes rebuild in the background while gets serve stale data, so tests poll until the +// refresh they triggered has landed. +async function waitForRefresh(probe: () => T | null | undefined): Promise { + const deadline = Date.now() + 5000; + for (;;) { + const value = probe(); + if (value !== null && value !== undefined) { + return value; + } + if (Date.now() > deadline) { + throw new Error('timed out waiting for background snapshot refresh'); + } + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + const PNG_1X1_BASE64 = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+nmX8AAAAASUVORK5CYII='; @@ -18,7 +34,7 @@ function makeTempDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-character-image-lookup-')); } -test('buildCharacterNameImageIndexFromSnapshots maps name terms to character portrait data URLs', () => { +test('buildCharacterNameImageIndexFromSnapshots maps name terms to character portrait data URLs', async () => { const outputDir = makeTempDir(); const snapshot: CharacterDictionarySnapshot = { formatVersion: CHARACTER_DICTIONARY_FORMAT_VERSION, @@ -75,9 +91,9 @@ test('buildCharacterNameImageIndexFromSnapshots maps name terms to character por { path: 'img/m130298-va456.png', dataBase64: 'BBBB' }, ], }; - writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); + await writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); - const index = buildCharacterNameImageIndexFromSnapshots(outputDir); + const index = await buildCharacterNameImageIndexFromSnapshots(outputDir); assert.deepEqual(index.get('アレクシア'), { src: 'data:image/png;base64,AAAA', @@ -85,7 +101,7 @@ test('buildCharacterNameImageIndexFromSnapshots maps name terms to character por }); }); -test('buildCharacterNameImageIndexFromSnapshots sniffs image MIME from bytes before path extension', () => { +test('buildCharacterNameImageIndexFromSnapshots sniffs image MIME from bytes before path extension', async () => { const outputDir = makeTempDir(); const snapshot: CharacterDictionarySnapshot = { formatVersion: CHARACTER_DICTIONARY_FORMAT_VERSION, @@ -116,14 +132,14 @@ test('buildCharacterNameImageIndexFromSnapshots sniffs image MIME from bytes bef ], images: [{ path: 'img/m130298-c123.jpg', dataBase64: PNG_1X1_BASE64 }], }; - writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); + await writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); - const index = buildCharacterNameImageIndexFromSnapshots(outputDir); + const index = await buildCharacterNameImageIndexFromSnapshots(outputDir); assert.equal(index.get('アレクシア')?.src, `data:image/png;base64,${PNG_1X1_BASE64}`); }); -test('createCharacterDictionaryImageLookup can scope duplicate names to the current media', () => { +test('createCharacterDictionaryImageLookup can scope duplicate names to the current media', async () => { const outputDir = makeTempDir(); const towerSnapshot: CharacterDictionarySnapshot = { formatVersion: CHARACTER_DICTIONARY_FORMAT_VERSION, @@ -173,15 +189,16 @@ test('createCharacterDictionaryImageLookup can scope duplicate names to the curr ], images: [{ path: 'img/m21202-c2.png', dataBase64: 'KONOSUBA' }], }; - writeSnapshot(getSnapshotPath(outputDir, towerSnapshot.mediaId), towerSnapshot); - writeSnapshot(getSnapshotPath(outputDir, konosubaSnapshot.mediaId), konosubaSnapshot); + await writeSnapshot(getSnapshotPath(outputDir, towerSnapshot.mediaId), towerSnapshot); + await writeSnapshot(getSnapshotPath(outputDir, konosubaSnapshot.mediaId), konosubaSnapshot); const lookup = createCharacterDictionaryImageLookup({ outputDir }); - assert.equal(lookup.get('カズ', 21202)?.alt, 'Kazuma'); + const scoped = await waitForRefresh(() => lookup.get('カズ', 21202)); + assert.equal(scoped.alt, 'Kazuma'); }); -test('createCharacterDictionaryImageLookup does not fall back globally on scoped miss', () => { +test('createCharacterDictionaryImageLookup does not fall back globally on scoped miss', async () => { const outputDir = makeTempDir(); const snapshot: CharacterDictionarySnapshot = { formatVersion: CHARACTER_DICTIONARY_FORMAT_VERSION, @@ -208,10 +225,11 @@ test('createCharacterDictionaryImageLookup does not fall back globally on scoped ], images: [{ path: 'img/m115230-c1.png', dataBase64: 'TOWER' }], }; - writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); + await writeSnapshot(getSnapshotPath(outputDir, snapshot.mediaId), snapshot); const lookup = createCharacterDictionaryImageLookup({ outputDir }); + const unscoped = await waitForRefresh(() => lookup.get('カズ')); + assert.equal(unscoped.alt, 'Kaz'); assert.equal(lookup.get('カズ', 21202), null); - assert.equal(lookup.get('カズ')?.alt, 'Kaz'); }); diff --git a/src/main/character-dictionary-runtime/image-lookup.ts b/src/main/character-dictionary-runtime/image-lookup.ts index 5bf89c79..d21e3020 100644 --- a/src/main/character-dictionary-runtime/image-lookup.ts +++ b/src/main/character-dictionary-runtime/image-lookup.ts @@ -204,11 +204,11 @@ function getSnapshotDirectorySignature(outputDir: string): string { return parts.sort().join('|'); } -export function buildCharacterNameImageIndexFromSnapshots( +export async function buildCharacterNameImageIndexFromSnapshots( outputDir: string, -): Map { +): Promise> { const index = new Map(); - for (const snapshot of readCachedSnapshots(outputDir)) { + for (const snapshot of await readCachedSnapshots(outputDir)) { appendSnapshotImages(index, snapshot); } return index; @@ -228,7 +228,12 @@ export function createCharacterDictionaryImageLookup(deps: { let signature: string | null = null; let index = new Map(); let indexByMediaId = new Map>(); + let refreshInFlight = false; + // Rebuilding means re-reading every cached snapshot (potentially GBs of JSON), which used to run + // synchronously inside a lookup and froze the whole app right after a snapshot changed. Lookups + // now serve the previous index while a single background rebuild catches up; the swap is atomic + // and the signature only advances once the rebuild it belongs to has landed. function refreshIfNeeded(): void { if (!outputDir) { index = new Map(); @@ -237,20 +242,30 @@ export function createCharacterDictionaryImageLookup(deps: { return; } const nextSignature = getSnapshotDirectorySignature(outputDir); - if (nextSignature === signature) { + if (nextSignature === signature || refreshInFlight) { return; } - signature = nextSignature; - index = new Map(); - indexByMediaId = new Map>(); - for (const snapshot of readCachedSnapshots(outputDir)) { - appendSnapshotImages(index, snapshot); - const mediaIndex = new Map(); - appendSnapshotImages(mediaIndex, snapshot); - if (mediaIndex.size > 0) { - indexByMediaId.set(snapshot.mediaId, mediaIndex); + refreshInFlight = true; + void (async () => { + try { + const snapshots = await readCachedSnapshots(outputDir); + const nextIndex = new Map(); + const nextIndexByMediaId = new Map>(); + for (const snapshot of snapshots) { + appendSnapshotImages(nextIndex, snapshot); + const mediaIndex = new Map(); + appendSnapshotImages(mediaIndex, snapshot); + if (mediaIndex.size > 0) { + nextIndexByMediaId.set(snapshot.mediaId, mediaIndex); + } + } + index = nextIndex; + indexByMediaId = nextIndexByMediaId; + signature = nextSignature; + } finally { + refreshInFlight = false; } - } + })(); } return { diff --git a/src/main/character-dictionary-runtime/name-candidates.test.ts b/src/main/character-dictionary-runtime/name-candidates.test.ts index e01e5a16..416b31ee 100644 --- a/src/main/character-dictionary-runtime/name-candidates.test.ts +++ b/src/main/character-dictionary-runtime/name-candidates.test.ts @@ -32,17 +32,33 @@ function writeSnapshot(outputDir: string, mediaId: number, entries: Array<[strin ); } -function withTempDir(run: (dir: string) => T): T { +async function withTempDir(run: (dir: string) => Promise | T): Promise { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-name-candidates-')); try { - return run(dir); + return await run(dir); } finally { fs.rmSync(dir, { recursive: true, force: true }); } } -test('collects terms and readings for the current media', () => { - withTempDir((dir) => { +// The snapshot index rebuilds in the background while lookups serve stale data, so tests poll the +// probe until the refresh they triggered has landed. +async function waitForRefresh(probe: () => T | null | undefined): Promise { + const deadline = Date.now() + 5000; + for (;;) { + const value = probe(); + if (value !== null && value !== undefined) { + return value; + } + if (Date.now() > deadline) { + throw new Error('timed out waiting for background snapshot refresh'); + } + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + +test('collects terms and readings for the current media', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [ ['ミナト', 'みなと'], ['湊', 'みなと'], @@ -53,17 +69,16 @@ test('collects terms and readings for the current media', () => { outputDir: dir, getCurrentMediaId: () => 1, }); - const candidates = lookup.get(); + const candidates = await waitForRefresh(() => lookup.get()); - assert.ok(candidates); assert.deepEqual([...candidates.forms].sort(), ['みなと', 'ミナト', '湊'].sort()); // Deduplicated: both entries share the みなと reading. assert.equal(candidates.forms.length, 3); }); }); -test('returns null without a media scope so the scanner stays exhaustive', () => { - withTempDir((dir) => { +test('returns null without a media scope so the scanner stays exhaustive', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [['ミナト', 'みなと']]); const lookup = createCharacterNameCandidateLookup({ @@ -71,12 +86,14 @@ test('returns null without a media scope so the scanner stays exhaustive', () => getCurrentMediaId: () => null, }); + // The explicitly-scoped probe proves the index has loaded before the unscoped case is judged. + await waitForRefresh(() => lookup.get(1)); assert.equal(lookup.get(), null); }); }); -test('returns null for a media with no cached snapshot', () => { - withTempDir((dir) => { +test('returns null for a media with no cached snapshot', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [['ミナト', 'みなと']]); const lookup = createCharacterNameCandidateLookup({ @@ -84,29 +101,31 @@ test('returns null for a media with no cached snapshot', () => { getCurrentMediaId: () => 999, }); + await waitForRefresh(() => lookup.get(1)); assert.equal(lookup.get(), null); }); }); -test('key changes when the snapshot content changes', () => { - withTempDir((dir) => { +test('key changes when the snapshot content changes', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [['ミナト', 'みなと']]); const lookup = createCharacterNameCandidateLookup({ outputDir: dir, getCurrentMediaId: () => 1, }); - const first = lookup.get(); + const first = await waitForRefresh(() => lookup.get()); writeSnapshot(dir, 1, [ ['ミナト', 'みなと'], ['アクア', 'あくあ'], ]); lookup.invalidate(); - const second = lookup.get(); + const second = await waitForRefresh(() => { + const candidates = lookup.get(); + return candidates && candidates.forms.length === 4 ? candidates : null; + }); - assert.ok(first && second); assert.notEqual(first.key, second.key); - assert.equal(second.forms.length, 4); }); }); @@ -114,8 +133,8 @@ test('key changes when the snapshot content changes', () => { // directory every call. Asserted behaviorally: an unannounced on-disk change is // invisible until the recheck interval elapses, which can only be true if the // filesystem is not consulted per lookup. -test('does not re-read the snapshot directory on every lookup', () => { - withTempDir((dir) => { +test('does not re-read the snapshot directory on every lookup', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [['ミナト', 'みなと']]); let nowMs = 1_000_000; const lookup = createCharacterNameCandidateLookup({ @@ -124,6 +143,7 @@ test('does not re-read the snapshot directory on every lookup', () => { now: () => nowMs, }); + await waitForRefresh(() => lookup.get()); assert.equal(lookup.get()?.forms.length, 2); writeSnapshot(dir, 1, [ @@ -135,12 +155,16 @@ test('does not re-read the snapshot directory on every lookup', () => { assert.equal(lookup.get()?.forms.length, 2, 'expected the cached list within the interval'); nowMs += 10_000; - assert.equal(lookup.get()?.forms.length, 4, 'expected a refresh past the interval'); + const refreshed = await waitForRefresh(() => { + const candidates = lookup.get(); + return candidates && candidates.forms.length === 4 ? candidates : null; + }); + assert.equal(refreshed.forms.length, 4, 'expected a refresh past the interval'); }); }); -test('invalidate picks up a snapshot change immediately', () => { - withTempDir((dir) => { +test('invalidate picks up a snapshot change on the next refresh', async () => { + await withTempDir(async (dir) => { writeSnapshot(dir, 1, [['ミナト', 'みなと']]); let nowMs = 1_000_000; const lookup = createCharacterNameCandidateLookup({ @@ -149,6 +173,7 @@ test('invalidate picks up a snapshot change immediately', () => { now: () => nowMs, }); + await waitForRefresh(() => lookup.get()); assert.equal(lookup.get()?.forms.length, 2); writeSnapshot(dir, 1, [ @@ -158,6 +183,10 @@ test('invalidate picks up a snapshot change immediately', () => { nowMs += 1; lookup.invalidate(); - assert.equal(lookup.get()?.forms.length, 4); + const refreshed = await waitForRefresh(() => { + const candidates = lookup.get(); + return candidates && candidates.forms.length === 4 ? candidates : null; + }); + assert.equal(refreshed.forms.length, 4); }); }); diff --git a/src/main/character-dictionary-runtime/name-candidates.ts b/src/main/character-dictionary-runtime/name-candidates.ts index 51de4aba..2de3cd5b 100644 --- a/src/main/character-dictionary-runtime/name-candidates.ts +++ b/src/main/character-dictionary-runtime/name-candidates.ts @@ -98,7 +98,12 @@ export function createCharacterNameCandidateLookup(deps: { let signature: string | null = null; let lastSignatureCheckAtMs = 0; let formsByMediaId = new Map(); + let refreshInFlight = false; + // Same stale-while-revalidate shape as the image lookup: the rebuild re-reads every cached + // snapshot, so it runs in the background while lookups keep serving the previous forms. The + // signature only advances once its rebuild has landed, so a failed or superseded rebuild is + // retried on the next signature check. function refreshIfNeeded(): void { if (!outputDir) { formsByMediaId = new Map(); @@ -114,17 +119,26 @@ export function createCharacterNameCandidateLookup(deps: { } lastSignatureCheckAtMs = nowMs; const nextSignature = getSnapshotDirectorySignature(outputDir); - if (nextSignature === signature) { + if (nextSignature === signature || refreshInFlight) { return; } - signature = nextSignature; - formsByMediaId = new Map(); - for (const snapshot of readCachedSnapshots(outputDir)) { - const forms = collectSnapshotNameForms(snapshot); - if (forms.length > 0) { - formsByMediaId.set(snapshot.mediaId, forms); + refreshInFlight = true; + void (async () => { + try { + const snapshots = await readCachedSnapshots(outputDir); + const nextFormsByMediaId = new Map(); + for (const snapshot of snapshots) { + const forms = collectSnapshotNameForms(snapshot); + if (forms.length > 0) { + nextFormsByMediaId.set(snapshot.mediaId, forms); + } + } + formsByMediaId = nextFormsByMediaId; + signature = nextSignature; + } finally { + refreshInFlight = false; } - } + })(); } return { diff --git a/src/main/character-dictionary-runtime/snapshot-refresh.test.ts b/src/main/character-dictionary-runtime/snapshot-refresh.test.ts index 47593d3c..b1d34fcf 100644 --- a/src/main/character-dictionary-runtime/snapshot-refresh.test.ts +++ b/src/main/character-dictionary-runtime/snapshot-refresh.test.ts @@ -34,7 +34,7 @@ function createSnapshotWithoutImages(): CharacterDictionarySnapshot { test('generateForCurrentMedia refreshes same-version snapshots missing images when inline images are enabled', async () => { const userDataPath = makeTempDir(); const outputDir = path.join(userDataPath, 'character-dictionaries'); - writeSnapshot(getSnapshotPath(outputDir, 130298), createSnapshotWithoutImages()); + await writeSnapshot(getSnapshotPath(outputDir, 130298), createSnapshotWithoutImages()); const originalFetch = globalThis.fetch; const fetchUrls: string[] = []; @@ -124,7 +124,7 @@ test('generateForCurrentMedia refreshes same-version snapshots missing images wh test('generateForCurrentMedia keeps failed MeCab name split refreshes retryable', async () => { const userDataPath = makeTempDir(); const outputDir = path.join(userDataPath, 'character-dictionaries'); - writeSnapshot(getSnapshotPath(outputDir, 130298), { + await writeSnapshot(getSnapshotPath(outputDir, 130298), { ...createSnapshotWithoutImages(), nameSplitSource: 'heuristic', }); @@ -213,7 +213,7 @@ test('generateForCurrentMedia keeps failed MeCab name split refreshes retryable' test('generateForCurrentMedia keeps mecab-split snapshots when MeCab is available', async () => { const userDataPath = makeTempDir(); const outputDir = path.join(userDataPath, 'character-dictionaries'); - writeSnapshot(getSnapshotPath(outputDir, 130298), { + await writeSnapshot(getSnapshotPath(outputDir, 130298), { ...createSnapshotWithoutImages(), nameSplitSource: 'mecab', }); @@ -253,7 +253,7 @@ test('generateForCurrentMedia keeps mecab-split snapshots when MeCab is availabl test('generateForCurrentMedia keeps heuristic-split snapshots while MeCab is unavailable', async () => { const userDataPath = makeTempDir(); const outputDir = path.join(userDataPath, 'character-dictionaries'); - writeSnapshot(getSnapshotPath(outputDir, 130298), { + await writeSnapshot(getSnapshotPath(outputDir, 130298), { ...createSnapshotWithoutImages(), nameSplitSource: 'heuristic', }); @@ -293,7 +293,7 @@ test('generateForCurrentMedia keeps heuristic-split snapshots while MeCab is una test('generateForCurrentMedia keeps same-version snapshots without images when inline images are disabled', async () => { const userDataPath = makeTempDir(); const outputDir = path.join(userDataPath, 'character-dictionaries'); - writeSnapshot(getSnapshotPath(outputDir, 130298), createSnapshotWithoutImages()); + await writeSnapshot(getSnapshotPath(outputDir, 130298), createSnapshotWithoutImages()); const originalFetch = globalThis.fetch; globalThis.fetch = (async (input: string | URL | Request) => { diff --git a/src/main/character-dictionary-runtime/zip.test.ts b/src/main/character-dictionary-runtime/zip.test.ts index e8fe7a49..dd257e97 100644 --- a/src/main/character-dictionary-runtime/zip.test.ts +++ b/src/main/character-dictionary-runtime/zip.test.ts @@ -42,7 +42,7 @@ function readStoredZipEntries(zipPath: string): Map { return entries; } -test('buildDictionaryZip writes a valid stored zip without fs.writeFileSync', () => { +test('buildDictionaryZip writes a valid stored zip without fs.writeFileSync', async () => { const tempDir = makeTempDir(); const outputPath = path.join(tempDir, 'dictionary.zip'); const termEntries: CharacterDictionaryTermEntry[] = [ @@ -62,7 +62,7 @@ test('buildDictionaryZip writes a valid stored zip without fs.writeFileSync', () ); }) as typeof Buffer.concat; - const result = buildDictionaryZip( + const result = await buildDictionaryZip( outputPath, 'Dictionary Title', 'Dictionary Description', @@ -106,11 +106,11 @@ test('buildDictionaryZip writes a valid stored zip without fs.writeFileSync', () } }); -test('readDictionaryZipRevision reads the built revision and rejects foreign archives', () => { +test('readDictionaryZipRevision reads the built revision and rejects foreign archives', async () => { const dir = makeTempDir(); try { const zipPath = path.join(dir, 'merged.zip'); - buildDictionaryZip( + await buildDictionaryZip( zipPath, 'SubMiner Character Dictionary', 'Character names', diff --git a/src/main/character-dictionary-runtime/zip.ts b/src/main/character-dictionary-runtime/zip.ts index 759a7ce6..9ab99604 100644 --- a/src/main/character-dictionary-runtime/zip.ts +++ b/src/main/character-dictionary-runtime/zip.ts @@ -1,5 +1,5 @@ import * as path from 'path'; -import { readStoredZipFirstFile, writeStoredZip } from '../../shared/stored-zip'; +import { readStoredZipFirstFile, writeStoredZipAsync } from '../../shared/stored-zip'; import { ensureDir } from './fs-utils'; import type { CharacterDictionarySnapshotImage, CharacterDictionaryTermEntry } from './types'; @@ -48,14 +48,14 @@ export function readDictionaryZipRevision(zipPath: string): string | null { } } -export function buildDictionaryZip( +export async function buildDictionaryZip( outputPath: string, dictionaryTitle: string, description: string, revision: string, termEntries: CharacterDictionaryTermEntry[], images: CharacterDictionarySnapshotImage[], -): { zipPath: string; entryCount: number } { +): Promise<{ zipPath: string; entryCount: number }> { ensureDir(path.dirname(outputPath)); function* zipFiles(): Iterable<{ name: string; data: Buffer }> { @@ -87,6 +87,6 @@ export function buildDictionaryZip( } } - writeStoredZip(outputPath, zipFiles()); + await writeStoredZipAsync(outputPath, zipFiles()); return { zipPath: outputPath, entryCount: termEntries.length }; } diff --git a/src/shared/stored-zip.ts b/src/shared/stored-zip.ts index 0e87df45..812c26d8 100644 --- a/src/shared/stored-zip.ts +++ b/src/shared/stored-zip.ts @@ -1,4 +1,5 @@ import * as fs from 'fs'; +import * as zlib from 'zlib'; type ZipEntry = { name: string; @@ -36,10 +37,17 @@ const CRC32_TABLE = (() => { return table; })(); +// Native CRC32 (Node >= 20.15) runs at native throughput, which matters for the multi-hundred-MB +// dictionary archives; the table loop stays as a fallback for runtimes without it. +const nativeCrc32 = (zlib as { crc32?: (data: Uint8Array, value?: number) => number }).crc32; + function crc32(data: Buffer): number { + if (typeof nativeCrc32 === 'function') { + return nativeCrc32(data) >>> 0; + } let crc = 0xffffffff; - for (const byte of data) { - crc = CRC32_TABLE[(crc ^ byte) & 0xff]! ^ (crc >>> 8); + for (let i = 0; i < data.length; i += 1) { + crc = CRC32_TABLE[(crc ^ data[i]!) & 0xff]! ^ (crc >>> 8); } return (crc ^ 0xffffffff) >>> 0; } @@ -294,59 +302,70 @@ function writeBuffer(fd: number, buffer: Buffer): void { } } +type ZipWriteState = { + entries: ZipEntry[]; + offset: number; +}; + +/** Appends one stored entry (local header + data) and returns the bytes written. */ +function appendStoredZipFile(fd: number, state: ZipWriteState, file: StoredZipFile): number { + const fileName = Buffer.from(file.name, 'utf8'); + const fileSize = file.data.length; + if (fileName.length > ZIP32_MAX_UINT16) { + throw new RangeError(`ZIP entry name too long: ${file.name}`); + } + if (fileSize > ZIP32_MAX_UINT32) { + throw new RangeError(`ZIP entry too large for ZIP32: ${file.name}`); + } + if (state.offset > ZIP32_MAX_UINT32) { + throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); + } + const fileCrc32 = crc32(file.data); + const localHeader = createLocalFileHeader(fileName, fileCrc32, fileSize); + const nextOffset = state.offset + localHeader.length + fileSize; + if (nextOffset > ZIP32_MAX_UINT32) { + throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); + } + writeBuffer(fd, localHeader); + writeBuffer(fd, file.data); + state.entries.push({ + name: file.name, + crc32: fileCrc32, + size: fileSize, + localHeaderOffset: state.offset, + }); + const written = nextOffset - state.offset; + state.offset = nextOffset; + return written; +} + +function finishStoredZip(fd: number, state: ZipWriteState): void { + const centralStart = state.offset; + if (centralStart > ZIP32_MAX_UINT32) { + throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); + } + for (const entry of state.entries) { + const centralHeader = createCentralDirectoryHeader(entry); + writeBuffer(fd, centralHeader); + state.offset += centralHeader.length; + } + + const centralSize = state.offset - centralStart; + writeBuffer(fd, createEndOfCentralDirectory(state.entries.length, centralSize, centralStart)); +} + export function writeStoredZip( outputPath: string, files: Iterable, ): { entryCount: number } { - const entries: ZipEntry[] = []; - let offset = 0; + const state: ZipWriteState = { entries: [], offset: 0 }; const fd = fs.openSync(outputPath, 'w'); try { for (const file of files) { - const fileName = Buffer.from(file.name, 'utf8'); - const fileSize = file.data.length; - if (fileName.length > ZIP32_MAX_UINT16) { - throw new RangeError(`ZIP entry name too long: ${file.name}`); - } - if (fileSize > ZIP32_MAX_UINT32) { - throw new RangeError(`ZIP entry too large for ZIP32: ${file.name}`); - } - if (offset > ZIP32_MAX_UINT32) { - throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); - } - const fileCrc32 = crc32(file.data); - const localHeader = createLocalFileHeader(fileName, fileCrc32, fileSize); - const nextOffset = offset + localHeader.length + fileSize; - if (nextOffset > ZIP32_MAX_UINT32) { - throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); - } - writeBuffer(fd, localHeader); - writeBuffer(fd, file.data); - entries.push({ - name: file.name, - crc32: fileCrc32, - size: fileSize, - localHeaderOffset: offset, - }); - if (nextOffset > ZIP32_MAX_UINT32) { - throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); - } - offset = nextOffset; + appendStoredZipFile(fd, state, file); } - - const centralStart = offset; - if (centralStart > ZIP32_MAX_UINT32) { - throw new RangeError('Archive exceeds ZIP32 limits (Zip64 not implemented)'); - } - for (const entry of entries) { - const centralHeader = createCentralDirectoryHeader(entry); - writeBuffer(fd, centralHeader); - offset += centralHeader.length; - } - - const centralSize = offset - centralStart; - writeBuffer(fd, createEndOfCentralDirectory(entries.length, centralSize, centralStart)); + finishStoredZip(fd, state); } catch (error) { fs.closeSync(fd); fs.rmSync(outputPath, { force: true }); @@ -354,5 +373,42 @@ export function writeStoredZip( } fs.closeSync(fd); - return { entryCount: entries.length }; + return { entryCount: state.entries.length }; +} + +// Yielding roughly every 8MB keeps individual event-loop blocks in the low tens of milliseconds +// while adding a negligible number of macrotask hops even for the largest merged dictionary. +const ASYNC_ZIP_YIELD_BYTE_BUDGET = 8 * 1024 * 1024; + +/** + * Same archive as {@link writeStoredZip}, written without starving the event loop: entry + * generation, CRC, and writes proceed in byte-budgeted slices with a macrotask yield in between. + * Multi-hundred-MB dictionary archives previously blocked the main process long enough for the + * compositor to declare the app unresponsive. + */ +export async function writeStoredZipAsync( + outputPath: string, + files: Iterable, +): Promise<{ entryCount: number }> { + const state: ZipWriteState = { entries: [], offset: 0 }; + const fd = fs.openSync(outputPath, 'w'); + + try { + let bytesSinceYield = 0; + for (const file of files) { + bytesSinceYield += appendStoredZipFile(fd, state, file); + if (bytesSinceYield >= ASYNC_ZIP_YIELD_BYTE_BUDGET) { + bytesSinceYield = 0; + await new Promise((resolve) => setImmediate(resolve)); + } + } + finishStoredZip(fd, state); + } catch (error) { + fs.closeSync(fd); + fs.rmSync(outputPath, { force: true }); + throw error; + } + + fs.closeSync(fd); + return { entryCount: state.entries.length }; }