diff --git a/packages/chord/CHANGELOG.md b/packages/chord/CHANGELOG.md index bbd4759bc..e8b74b5e9 100644 --- a/packages/chord/CHANGELOG.md +++ b/packages/chord/CHANGELOG.md @@ -17,6 +17,10 @@ - `ownedSlice(text, start, end?)` in `@earendil-works/chord/delta`: a slice of a string that does not keep the whole string alive ([#9265](https://github.com/earendil-works/pi/issues/9265)) - `appendText(draft, key, text)` in `@earendil-works/chord/delta`: append to a string in a draft at O(text) cost, recorded as an append op without comparing the old and new strings (a plain `draft[key] += text` makes `prepare()` read the whole string, O(length) per change); `StringKeys` names the keys it accepts ([#9265](https://github.com/earendil-works/pi/issues/9265)) +### Changed + +- `applyImmutable()`, `applyImmutableBatches()` and tracker revisions copy each container on a changed path with spread (or `Object.assign()` onto a null-prototype object) instead of one `Object.defineProperty()` per key, about 25 times faster per container ([#9265](https://github.com/earendil-works/pi/issues/9265)) + ### Fixed - Fixed append ops from the delta tracker and `diffRevisions()` keeping the whole string they were cut from alive (V8 represents a slice as a view of its source), so a delta chain, a watch backlog or a replica built from appends to a growing text held one copy of the text per commit ([#9265](https://github.com/earendil-works/pi/issues/9265)) diff --git a/packages/chord/src/delta/apply-immutable-trusted.ts b/packages/chord/src/delta/apply-immutable-trusted.ts index 7944bfae1..cde0300e7 100644 --- a/packages/chord/src/delta/apply-immutable-trusted.ts +++ b/packages/chord/src/delta/apply-immutable-trusted.ts @@ -86,12 +86,9 @@ function copyPath(root: JsonValue, path: Path, length: number, owned: WeakSet; - for (const key of Object.keys(value)) defineData(result, key, value[key]!); - return result; + // Spread and `Object.assign()` onto a null-prototype object define own data properties, `__proto__` included, as + // `defineData()` per key does, at a fraction of its cost. + return Object.getPrototypeOf(value) === null ? Object.assign(Object.create(null), value) : { ...value }; } function read(target: JsonContainer, key: Seg): JsonValue { diff --git a/packages/chord/src/delta/index.ts b/packages/chord/src/delta/index.ts index f2ec7ec53..1af25be29 100644 --- a/packages/chord/src/delta/index.ts +++ b/packages/chord/src/delta/index.ts @@ -490,19 +490,9 @@ function copyContainers(root: JsonValue, path: Path, owned: WeakSet): Js const copy = (value: JsonValue): JsonValue[] | Record => { if (Array.isArray(value)) return value.slice(); if (!isObj(value)) throw new PathError(path); - const result = Object.create(Object.getPrototypeOf(value) === null ? null : Object.prototype) as Record< - string, - JsonValue - >; - for (const key of Object.keys(value)) { - Object.defineProperty(result, key, { - value: (value as Record)[key], - writable: true, - enumerable: true, - configurable: true, - }); - } - return result; + // Spread and `Object.assign()` onto a null-prototype object define own data properties, `__proto__` included, + // as `Object.defineProperty()` per key does, at a fraction of its cost. + return Object.getPrototypeOf(value) === null ? Object.assign(Object.create(null), value) : { ...value }; }; if (!isObj(root)) throw new PathError(path); let copiedRoot: JsonValue[] | Record; diff --git a/packages/chord/test/delta-apply-immutable.test.ts b/packages/chord/test/delta-apply-immutable.test.ts index ff6f1de0d..d00ef0e10 100644 --- a/packages/chord/test/delta-apply-immutable.test.ts +++ b/packages/chord/test/delta-apply-immutable.test.ts @@ -246,6 +246,30 @@ describe("checked immutable operation application", () => { }); }); +describe("immutable container copies", () => { + it("keep an own __proto__ member as data and the prototype of the copied object", () => { + const plain = JSON.parse('{"__proto__":{"polluted":true},"n":1}') as Record; + const bare = Object.assign(Object.create(null) as Record, { n: 1 }); + for (const base of [plain, bare]) { + const checked = applyImmutable(base, [["s", ["n"], 2]]); + // Tracker revisions copy through the trusted path. + const tracker = track(base); + const change = tracker.beginChange(); + change.state.n = 2; + tracker.adopt(change.prepare()); + for (const copy of [checked, tracker.value]) { + expect(copy).not.toBe(base); + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf(base)); + expect(Object.keys(copy)).toEqual(Object.keys(base)); + expect(copy.n).toBe(2); + if (base === plain) + expect(Object.getOwnPropertyDescriptor(copy, "__proto__")?.value).toEqual({ polluted: true }); + } + } + expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); + }); +}); + describe("immutable application complexity", () => { it("appends to an array not yet copied in one copy, sharing everything else", () => { const entries = Array.from({ length: 1_000 }, (_, index) => ({ index })); @@ -291,18 +315,15 @@ describe("immutable application complexity", () => { const base: Record = {}; for (let index = 0; index < width; index++) base[`field${index}`] = index; const operations: Op[] = Array.from({ length: 1_000 }, (_, index): Op => ["s", [`field${index}`], -index]); - const originalKeys = Object.keys; + // A copy lists the object's keys; the proxy counts the listings. let shallowCopies = 0; - Object.keys = ((value: object): string[] => { - shallowCopies++; - return originalKeys(value); - }) as typeof Object.keys; - let result: Record; - try { - result = applyImmutable(base, operations); - } finally { - Object.keys = originalKeys; - } + const counted = new Proxy(base, { + ownKeys(target) { + shallowCopies++; + return Reflect.ownKeys(target); + }, + }); + const result = applyImmutable(counted, operations); expect(result.field999).toBe(-999); expect(base.field999).toBe(999); diff --git a/packages/chord/test/delta-tracker/retention.test.ts b/packages/chord/test/delta-tracker/retention.test.ts index 29543857d..fe39b0dd0 100644 --- a/packages/chord/test/delta-tracker/retention.test.ts +++ b/packages/chord/test/delta-tracker/retention.test.ts @@ -18,6 +18,7 @@ describe("delta tracker retention across GC jobs", () => { "same-job-fast-cleanup", "same-job-folded-ops-cleanup", "lifecycle-churn", + "adopted-changes-flat", ])( "validates %s retention semantics", (scenario) => { diff --git a/packages/chord/test/delta-tracker/retention.worker.ts b/packages/chord/test/delta-tracker/retention.worker.ts index a0a80d8d4..23e9efa85 100644 --- a/packages/chord/test/delta-tracker/retention.worker.ts +++ b/packages/chord/test/delta-tracker/retention.worker.ts @@ -260,6 +260,35 @@ if (scenario === "settled-lifecycle") { const retainedMiB = (process.memoryUsage().heapUsed - readyHeap) / (1024 * 1024); assert.ok(retainedMiB < 8, `same-job folded operations retained ${retainedMiB.toFixed(2)} MiB`); assert.equal(tracker.value.field0, 100); +} else if (scenario === "adopted-changes-flat") { + // Each change registers its context with the tracker through a WeakRef; adoption must drop every registration, so + // a long run of small changes keeps only the document, not a record per change. + const gc = global.gc; + assert.ok(gc, "worker requires --expose-gc"); + const tracker = track({ entries: [{ id: 0, text: "" }] }); + const commit = (): void => { + const change = tracker.beginChange(); + change.state.entries[0]!.text += " tok"; + tracker.adopt(change.prepare()); + }; + const settledHeap = async (): Promise => { + for (let round = 0; round < 5; round++) { + await setImmediate(); + gc(); + } + return process.memoryUsage().heapUsed; + }; + for (let index = 0; index < 1_000; index++) commit(); + const readyHeap = await settledHeap(); + for (let index = 0; index < 10_000; index++) { + commit(); + // Let WeakRef targets go, as between the jobs of real commits. + if (index % 100 === 0) await setImmediate(); + } + // The text itself grows 4 bytes per change. + const perChange = ((await settledHeap()) - readyHeap) / 10_000 - 4; + assert.ok(perChange < 48, `each adopted change retained ${perChange.toFixed(1)} bytes`); + assert.equal(tracker.value.entries[0]!.text.length, 4 * 11_000); } else if (scenario === "lifecycle-churn") { const tracker = track({ value: 0 }); for (let index = 0; index < 100_000; index++) {