Skip to content

Commit 7fba664

Browse files
author
Fred Wu
committed
fix(plots): preserve cached history during replacement and startup cleanup
Reclaim superseded payloads before evicting retained plots. Observe JGD history changes before asynchronous httpgd initialization. Add regression coverage for full-quota replacement and R exit during startup.
1 parent 3ec8a47 commit 7fba664

4 files changed

Lines changed: 119 additions & 13 deletions

File tree

‎src/plotViewer/index.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,8 @@ export class CommonPlotManager implements PlotManager {
130130
!this.historyCheckpointId ||
131131
trimmed ||
132132
saved?.sessions.length !== this.jgdManager.history.sessionIds().length;
133-
const sources = extensionContext.workspaceState.get<unknown>('r.plot.standardSessions');
134-
this.restoredStandardSessions = Array.isArray(sources)
135-
? sources.filter(isViewerSessionSource)
136-
: [];
137-
for (const source of this.restoredStandardSessions) {
138-
this.lifecycle.observe(source);
139-
}
140-
await this.httpgdManager.initialize();
133+
// Restored owners are already observed. Track exits before another
134+
// backend's asynchronous initialization can yield to those callbacks.
141135
this.historySubscription = this.jgdManager.history.onDidChange((_id, selectionOnly) => {
142136
this.historyContentDirty ||= !selectionOnly;
143137
this.historySelectionDirty = true;
@@ -150,6 +144,14 @@ export class CommonPlotManager implements PlotManager {
150144
}, 250);
151145
}
152146
});
147+
const sources = extensionContext.workspaceState.get<unknown>('r.plot.standardSessions');
148+
this.restoredStandardSessions = Array.isArray(sources)
149+
? sources.filter(isViewerSessionSource)
150+
: [];
151+
for (const source of this.restoredStandardSessions) {
152+
this.lifecycle.observe(source);
153+
}
154+
await this.httpgdManager.initialize();
153155

154156
for (const cmd of commands) {
155157
const fullCommand = `r.plot.${cmd}`;

‎src/plotViewer/persistentPlotCache.ts‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,22 @@ export class PersistentPlotCache {
9090
}
9191
}
9292

93-
private async makeRoom(bytes: number): Promise<void> {
93+
private async makeRoom(bytes: number, retained?: ReadonlySet<string>): Promise<void> {
94+
// Replacements must release superseded payloads before evicting plots
95+
// still referenced by the next checkpoint or the other backend.
96+
if (retained) {
97+
for (const key of this.files.keys()) {
98+
if (this.bytes + bytes <= this.maxBytes) {
99+
break;
100+
}
101+
if (!retained.has(key)) {
102+
await this.remove(key);
103+
}
104+
}
105+
}
106+
if (this.bytes + bytes <= this.maxBytes) {
107+
return;
108+
}
94109
const oldest = [...this.files].sort((a, b) => a[1].created - b[1].created);
95110
for (const [key] of oldest) {
96111
if (this.bytes + bytes <= this.maxBytes) {
@@ -138,7 +153,10 @@ export class PersistentPlotCache {
138153
const operation = this.writes.then(async () => {
139154
await this.initialize();
140155
const references = new Set<string>();
141-
const metadata = await build(async (plot) => {
156+
// Keep snapshot objects, not a second serialized copy of the entire
157+
// history. Only missing payloads need serialization again for writing.
158+
const pending = new Map<string, { plot: object; bytes: number }>();
159+
const metadata = await build((plot) => {
142160
const content = JSON.stringify(plot);
143161
const key = createHash('sha256').update(content).digest('hex');
144162
references.add(key);
@@ -149,7 +167,22 @@ export class PersistentPlotCache {
149167
!this.files.has(key) &&
150168
!this.discarded.has(key)
151169
) {
152-
await this.makeRoom(bytes);
170+
pending.set(key, { plot, bytes });
171+
}
172+
return Promise.resolve({ cacheKey: key });
173+
});
174+
const retained = new Set(references);
175+
for (const [other, keys] of this.references) {
176+
if (other !== namespace) {
177+
for (const key of keys) {
178+
retained.add(key);
179+
}
180+
}
181+
}
182+
for (const [key, { plot, bytes }] of pending) {
183+
if (this.directory) {
184+
const content = JSON.stringify(plot);
185+
await this.makeRoom(bytes, retained);
153186
const filename = path.join(this.directory, `${key}.json`);
154187
try {
155188
await fs.writeFile(filename, content);
@@ -160,8 +193,7 @@ export class PersistentPlotCache {
160193
this.files.set(key, { bytes, created: Date.now() });
161194
this.bytes += bytes;
162195
}
163-
return { cacheKey: key };
164-
});
196+
}
165197
await publish(metadata);
166198
this.register(namespace, references);
167199
await this.collect();

‎src/test/node/persistentPlotCache.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,43 @@ suite('Persistent workspace plot cache', () => {
9090
assert.equal(plot.data.length, 50);
9191
});
9292

93+
test('an equal-sized replacement at full quota preserves unchanged history across reload', async () => {
94+
const a = { id: '1', data: '<svg>A</svg>', width: 800, height: 600, zoom: 1 };
95+
const b = { ...a, id: '2', data: '<svg>B</svg>' };
96+
const replacement = { ...b, data: '<svg>C</svg>' };
97+
const bytes = Buffer.byteLength(JSON.stringify(a));
98+
const cache = new PersistentPlotCache(directory, bytes * 2);
99+
const original = await put(cache, 'httpgd', [a, b]);
100+
const updated = await put(cache, 'httpgd', [a, replacement]);
101+
102+
assert.equal(await size(), bytes * 2);
103+
assert.deepEqual(await cache.read(updated[0]), a);
104+
assert.deepEqual(await cache.read(updated[1]), replacement);
105+
assert.equal(await cache.read(original[1]), undefined);
106+
const reload = new PersistentPlotCache(directory, bytes * 2);
107+
reload.register(
108+
'httpgd',
109+
updated.map((ref) => ref.cacheKey),
110+
);
111+
assert.deepEqual(await reload.read(updated[0]), a);
112+
assert.deepEqual(await reload.read(updated[1]), replacement);
113+
});
114+
115+
test('replacement preserves payloads still referenced by another backend', async () => {
116+
const a = { data: 'a' },
117+
b = { data: 'b' },
118+
replacement = { data: 'c' };
119+
const bytes = Buffer.byteLength(JSON.stringify(a));
120+
const cache = new PersistentPlotCache(directory, bytes * 3);
121+
const shared = await put(cache, 'jgd', [b]);
122+
await put(cache, 'httpgd', [a, b]);
123+
const updated = await put(cache, 'httpgd', [a, replacement]);
124+
assert.equal(await size(), bytes * 3);
125+
assert.deepEqual(await cache.read(shared[0]), b);
126+
assert.deepEqual(await cache.read(updated[0]), a);
127+
assert.deepEqual(await cache.read(updated[1]), replacement);
128+
});
129+
93130
test('large histories share the byte budget without rewriting unchanged files', async () => {
94131
const cache = new PersistentPlotCache(directory, 20 * 1024 * 1024);
95132
const a = { data: 'a'.repeat(8 * 1024 * 1024) };

‎src/test/plot/plotRestoration.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,41 @@ suite('Standalone plot restoration', () => {
468468
assert.deepStrictEqual((state.get('r.plot.jgdState') as PlotHistoryState).sessions, []);
469469
});
470470

471+
test('JGD exit during httpgd initialization removes its saved checkpoint and payload', async () => {
472+
const exits = processExits();
473+
const source = owner('initializing-exit', '123');
474+
const first = await manager();
475+
first.jgdManager.history.addPlot('device', frame('device'));
476+
first.jgdManager.history.setSessionSource('device', source);
477+
await first.dispose();
478+
const cacheDirectory = path.join(storageDirectory, 'plot-cache');
479+
assert.strictEqual((await fs.readdir(cacheDirectory)).length, 1);
480+
481+
const restored = new CommonPlotManager();
482+
managers.push(restored);
483+
const started = deferred<void>();
484+
const gate = deferred<void>();
485+
const initialize = restored.httpgdManager.initialize.bind(restored.httpgdManager);
486+
sandbox.stub(restored.httpgdManager, 'initialize').callsFake(async () => {
487+
started.resolve();
488+
await gate.promise;
489+
await initialize();
490+
});
491+
const initializing = restored.initialize();
492+
try {
493+
await started.promise;
494+
assert.strictEqual(restored.jgdManager.history.count('device'), 1);
495+
exits.exit(source);
496+
assert.deepStrictEqual(restored.jgdManager.history.sessionIds(), []);
497+
} finally {
498+
gate.resolve();
499+
await initializing;
500+
}
501+
await restored.dispose();
502+
assert.deepStrictEqual((state.get('r.plot.jgdState') as PlotHistoryState).sessions, []);
503+
assert.deepStrictEqual(await fs.readdir(cacheDirectory), []);
504+
});
505+
471506
test('httpgd process exit clears history, connections and saved credentials without affecting other owners', async () => {
472507
const exits = processExits();
473508
const source = owner('httpgd-owner', '14001');

0 commit comments

Comments
 (0)