From 044fd70462ffaf72ce25666e80cc1e397a95fc61 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Wed, 18 Mar 2026 11:47:37 +1100 Subject: [PATCH 1/5] remove explicit FloorData --- client/bin/app.ts | 54 ++++++------ client/lib/gumnut/floor.ts | 142 ++++++++++++++++++++------------ tests/integration/basic.test.ts | 42 +++++----- 3 files changed, 133 insertions(+), 105 deletions(-) diff --git a/client/bin/app.ts b/client/bin/app.ts index 820a999..e95b9a4 100755 --- a/client/bin/app.ts +++ b/client/bin/app.ts @@ -20,24 +20,18 @@ let cursorAt: InoPosRef | null = null; function reconcileOtherCursors() { // TODO: replace other user cursors const cursors = floor.byIno(FIXED_CURSORS); - const selfCursorPos = cursorAt ? floor.resolveRef(cursorAt) : 0; + const selfCursorIndex = (cursorAt ? floor.resolveRef(cursorAt) : 0) - 1; const cursorsToRender = new Map(); + const data = cursors.data(); - for (let i = 0; i < cursors.length(); ++i) { - if (selfCursorPos - 1 === i) { - continue; - } - // TODO: someone else - - const v = cursors.at(i); - if (!(v instanceof InoPosRef)) { - continue; + data.forEach((v, index) => { + if (index === selfCursorIndex || !(v instanceof InoPosRef)) { + return; } - const pos = floor.resolveRef(v); - cursorsToRender.set(String(i), pos); - } + cursorsToRender.set(String(index), pos); + }); e.cursor = cursorsToRender; } @@ -45,12 +39,12 @@ function reconcileOtherCursors() { e.addListener( 'modify', (m) => { - floor.perform(function* (byIno) { - const api = byIno(FIXED_STRING); - api.updateString(m.lo, m.hi - m.lo, m.insert); + floor.perform(function* (t) { + const api = t.byIno(FIXED_STRING); + api.stringUpdate(m.lo, m.hi - m.lo, m.insert); }); - e.text = model.asString(); + e.text = model.string(); reconcileOtherCursors(); // readjust cursor and collapse selection @@ -72,19 +66,19 @@ e.addListener( e.addListener( 'cursor', () => { - floor.perform(function* (byIno, resolveRef) { - const c = byIno(FIXED_CURSORS); - const text = byIno(FIXED_STRING); + floor.perform(function* (t) { + const c = t.byIno(FIXED_CURSORS); + const text = t.byIno(FIXED_STRING); - let pos = cursorAt ? resolveRef(cursorAt) : 0; + let pos = cursorAt ? t.resolveRef(cursorAt) : 0; if (pos === 0) { // insert self - c.splice(0, 0, null); + c.dataUpdate(0, 0, null); cursorAt = c.refAt(1); // TODO: this is pos, not index pos = 1; } - c.set(pos - 1, text.refAt(e.pos)); + c.set(pos, text.refAt(e.pos)); }); }, c.signal, @@ -102,17 +96,17 @@ e.addListener( e.addListener( 'escape', () => { - floor.perform(function* (byIno) { - const api = byIno(FIXED_NUM); + floor.perform(function* (t) { + const api = t.byIno(FIXED_NUM); // this is a fairly basic transaction loop; if something changes, re-run for (;;) { if (api.length() === 0) { - api.splice(0, 0, 0); + api.dataUpdate(0, 0, 0); } - const last = api.at(0); + const last = api.at(0); // we just inserted ourself const update = typeof last === 'number' ? last + 1 : 1; - api.set(0, update); + api.set(1, update); console.warn('! setting num', update); // TODO: we're not announcing this in the client anywhere @@ -124,7 +118,7 @@ e.addListener( ); e.prompt = '> '; -e.text = model.asString(); +e.text = model.string(); const conn = connectToPulse(c.signal, { remote: 'http://localhost:8080/demo', @@ -170,7 +164,7 @@ for (;;) { floor.update(pack); // TODO: we assume FIXED_STRING changes (not always true) - e.text = model.asString(); + e.text = model.string(); reconcileOtherCursors(); e.pos = floor.resolveRef(cursor); diff --git a/client/lib/gumnut/floor.ts b/client/lib/gumnut/floor.ts index 6e063c1..7e124ad 100644 --- a/client/lib/gumnut/floor.ts +++ b/client/lib/gumnut/floor.ts @@ -197,16 +197,16 @@ export class GumnutFloor { length() { return internal.length(); }, - at(at: number): FloorData { - const int = internal.readData(at, at + 1)[0]; - if (int === emptySymbol) { - return undefined; - } else if (int instanceof GumnutInternString) { - return int.s; + at(index: number) { + if (index < 0) { + index = internal.length() + index; } - return int; + return internal.readData(index, index + 1)[0]; + }, + data(start, end) { + return internal.readData(start, end); }, - asString(start, end) { + string(start, end) { return internal.readString(start, end); }, refAt(at: number): InoPosRef { @@ -216,61 +216,51 @@ export class GumnutFloor { }; } - resolveRef(r: InoPosRef) { - const impl = r as InoPosRefImpl; - return this.low.byIno(impl.target).posOf(impl.id); - } - /** * Builds a {@link FloorApi} from the internal {@link PatchApi}. * * This just smooths over internal string/internal stuff. * TODO: can we not? */ - private toFloorApi(ino: string, internal: PatchApi): FloorApi { + private writeForPatchApi(ino: string, internal: PatchApi): FloorApi { return { length() { return internal.length(); }, - at(at: number): FloorData { - const int = internal.readData(at, at + 1)[0]; - if (int === emptySymbol) { - return undefined; - } else if (int instanceof GumnutInternString) { - return int.s; + at(index: number) { + if (index < 0) { + index = internal.length() + index; } - return int; + return internal.readData(index, index + 1)[0]; + }, + data(start, end) { + return internal.readData(start, end); }, - asString(start, end) { + string(start, end) { return internal.readString(start, end); }, refAt(at: number): InoPosRef { const id = internal.posToId(at); return new InoPosRefImpl(ino, id); }, - set(at: number, ...data: FloorData[]): void { - const before = at + data.length; - const int = data.map((v): GumnutData => { - if (typeof v === 'string') { - return internString(v); - } - return v; - }); - if (int.length) { - internal.applySet({ before, body: int }); - } + // TODO: above this is the same as byIno + + set(before: number, ...data: GumnutData[]): void { + internal.applySet({ before, body: data }); }, - splice(at: number, deleteCount: number, ...data: FloorData[]): void { + + dataUpdate(at, deleteCount, ...data) { if (deleteCount > 0) { internal.applyOp({ r: [at, at + deleteCount] }); } if (data.length) { internal.applyOp({ r: [at], length: data.length }); - this.set(at, ...data); + internal.applySet({ before: at + data.length, body: data }); } }, - updateString(at, deleteCount, str) { + + stringUpdate(at, deleteCount, str) { if (deleteCount > 0) { internal.applyOp({ r: [at, at + deleteCount] }); } @@ -282,37 +272,83 @@ export class GumnutFloor { }; } + resolveRef(r: InoPosRef) { + const impl = r as InoPosRefImpl; + return this.low.byIno(impl.target).posOf(impl.id); + } + perform(fn: FloorFn) { - const toFloorApi = this.toFloorApi.bind(this); + const writeForPatchApi = this.writeForPatchApi.bind(this); + this.low.perform(function* (byIno) { - yield* fn( - (ino) => toFloorApi(ino, byIno(ino)), - (r: InoPosRef) => { + const api: FloorInoApi = { + byIno(ino) { + const int = byIno(ino); + return writeForPatchApi(ino, int); + }, + resolveRef(r) { const impl = r as InoPosRefImpl; return byIno(impl.target).posOf(impl.id); }, - ); + }; + yield* fn(api); }); this.announceCh.push(true); } } -export type FloorFn = ( - byIno: (ino: string) => FloorApi, - resolveRef: (r: InoPosRef) => number, -) => Generator, void, void>; +export type FloorInoApi = { + byIno(ino: string): FloorApi; + resolveRef(r: InoPosRef): number; +}; + +export type FloorFn = (api: FloorInoApi) => Generator, void, void>; export type FloorReadApi = { - at(at: number): FloorData; - asString(start?: number, end?: number): string; - refAt(at: number): InoPosRef; + /** + * Returns the length of this inode. + */ length(): number; + + /** + * Returns the data at a given position, with {@link Array.at}-like semantics. + */ + at(index: number): GumnutData; + + /** + * Reads the range of data here as {@link GumnutData}. + */ + data(start?: number, end?: number): GumnutData[]; + + /** + * Reads the range of data here as a string. + */ + string(start?: number, end?: number): string; + + /** + * Converts this index to a {@link InoPosRef}. + * + * This index must be in the range `[0,length]`, and this will throw otherwise. + * If this is to track an _item_, you should use the index after that item, as it will be consistent over time. + */ + refAt(at: number): InoPosRef; }; export type FloorApi = FloorReadApi & { - set(at: number, ...data: FloorData[]): void; - splice(at: number, deleteCount: number, ...data: FloorData[]): void; - updateString(at: number, deleteCount: number, str: string): void; -}; + /** + * Set replaces data _before_ the given position (1-indexed). + * + * To replace index 0, call `.set(1, ...)`. + */ + set(before: number, ...data: GumnutData[]): void; -export type FloorData = null | undefined | boolean | number | bigint | string | InoPosRef; + /** + * Acts like 'splice', merging delete/insert operations for data. + */ + dataUpdate(at: number, deleteCount: number, ...data: GumnutData[]): void; + + /** + * Acts like 'splice', merging delete/insert operations for string data. + */ + stringUpdate(at: number, deleteCount: number, str: string): void; +}; diff --git a/tests/integration/basic.test.ts b/tests/integration/basic.test.ts index 454b89d..e584f9b 100644 --- a/tests/integration/basic.test.ts +++ b/tests/integration/basic.test.ts @@ -2,36 +2,34 @@ import { test } from 'node:test'; import * as assert from 'node:assert'; import { connectToGumnutLow } from './util.ts'; import { checkCond } from 'thorish'; -import { InoPosRef } from '../../client/lib/gumnut/types.ts'; -import type { FloorData } from '../../client/lib/gumnut/floor.ts'; +import { type GumnutData, InoPosRef } from '../../client/lib/gumnut/types.ts'; +import { internString } from '../../client/lib/gumnut/intern.ts'; test('basic', async (t) => { const doc1 = await connectToGumnutLow(t.signal); const doc2 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); - doc1.floor.perform(function* (byIno) { - const api = byIno('abc'); - api.splice(0, 0, null, null, null, null, null, null, null); // x7 + doc1.floor.perform(function* (t) { + const api = t.byIno('abc'); + api.dataUpdate(0, 0, ...Array(7).fill(null)); }); await checkCond(() => { assert.strictEqual(doc2.floor.byIno('abc').length(), 7); }); - doc2.floor.perform(function* (byIno) { - const api = byIno('abc'); - api.set(1, 'interned string', -100n, -Infinity, undefined); + doc2.floor.perform(function* (t) { + const api = t.byIno('abc'); + api.set(5, internString('interned string'), -100n, -Infinity, undefined); }); await checkCond(() => { const ino = doc1.floor.byIno('abc'); - const check: FloorData[] = []; - for (let i = 0; i < ino.length(); ++i) { - check.push(ino.at(i)); - } + + const check = ino.data(); assert.deepStrictEqual(check, [ null, - 'interned string', + internString('interned string'), -100n, -Infinity, undefined, @@ -45,18 +43,18 @@ test('naïve ref over time', async (t) => { const doc1 = await connectToGumnutLow(t.signal); // create initial state: 'foo' with 50 items, 'bar' containing single pointing back 10 - doc1.floor.perform(function* (byIno, resolveRef) { - const apiFoo = byIno('foo'); + doc1.floor.perform(function* (t) { + const apiFoo = t.byIno('foo'); const arr = new Array(50).fill(7); - apiFoo.splice(0, 0, ...arr); + apiFoo.dataUpdate(0, 0, ...arr); - const apiBar = byIno('bar'); - apiBar.splice(0, 0, apiFoo.refAt(40)); + const apiBar = t.byIno('bar'); + apiBar.dataUpdate(0, 0, apiFoo.refAt(40)); const wasRef = apiBar.at(0); assert.ok(wasRef instanceof InoPosRef); - assert.strictEqual(resolveRef(wasRef), 40); + assert.strictEqual(t.resolveRef(wasRef), 40); }); // check resolved pos on new conn @@ -71,9 +69,9 @@ test('naïve ref over time', async (t) => { }); // remove some data, in theory shifting ref back too - doc2.floor.perform(function* (byIno) { - const apiFoo = byIno('foo'); - apiFoo.splice(0, 25); + doc2.floor.perform(function* (t) { + const apiFoo = t.byIno('foo'); + apiFoo.dataUpdate(0, 25); }); // check resolved pos From 828d7b98b3b1b4e2975fd270e419b3d1f4077dc4 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Wed, 18 Mar 2026 11:47:45 +1100 Subject: [PATCH 2/5] check script --- scripts/check.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100755 scripts/check.sh diff --git a/scripts/check.sh b/scripts/check.sh new file mode 100755 index 0000000..7253a9b --- /dev/null +++ b/scripts/check.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +set -eu + +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) +cd $SCRIPT_DIR/.. + +echo "Source" +GO_LINES=$(cat $(find ./ -name \*\.go) | wc -l) +TS_LINES=$(cat $(find ./ -name node_modules -prune -o -name \*\.ts -print) | wc -l) +echo "${GO_LINES} Go lines" +echo "${TS_LINES} TS lines" + +echo "Go test" +TIMEFORMAT='--Go test: %R sec' +time { + go test ./... -cover +} + +echo "TS client test" +TIMEFORMAT='--TS test: %R sec' +time { + pnpm test:client +} + +echo "Integration test" +TIMEFORMAT='--Integration test: %R sec' +time { + pnpm run test:integration +} + +# echo "TS types" +# TIMEFORMAT='--TS types: %R sec' +# time { +# pnpm run test:typecheck +# } + +# echo "API build" +# TIMEFORMAT='--API build: %R sec' +# time { +# pnpm build:api +# } From aaa5c9d00d06365e7068082679032cc01442d857 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Wed, 18 Mar 2026 13:53:51 +1100 Subject: [PATCH 3/5] remove intern string, use real string --- client/lib/gumnut/floor.ts | 12 ++-- client/lib/gumnut/intern.test.ts | 8 --- client/lib/gumnut/intern.ts | 42 -------------- client/lib/gumnut/types.ts | 3 +- client/lib/ymodel/api.test.ts | 12 ++-- client/lib/ymodel/internal/state.test.ts | 3 +- client/lib/ymodel/internal/state.ts | 70 ++++++------------------ client/lib/ymodel/types.ts | 49 ++++++++++------- client/lib/ymodel/work-holder.test.ts | 20 +++++-- client/lib/ymodel/work-holder.ts | 60 ++++++++------------ tests/integration/basic.test.ts | 7 +-- 11 files changed, 103 insertions(+), 183 deletions(-) delete mode 100644 client/lib/gumnut/intern.test.ts delete mode 100644 client/lib/gumnut/intern.ts diff --git a/client/lib/gumnut/floor.ts b/client/lib/gumnut/floor.ts index 7e124ad..9177731 100644 --- a/client/lib/gumnut/floor.ts +++ b/client/lib/gumnut/floor.ts @@ -11,7 +11,6 @@ import { type WirePatch, } from '../ymodel/types.ts'; import { emptySymbol, InoPosRef, InoPosRefImpl, type GumnutData } from './types.ts'; -import { GumnutInternString, internString } from './intern.ts'; import type { PatchApi } from '../ymodel/work-holder.ts'; type WireType = { @@ -20,8 +19,11 @@ type WireType = { }; class GumnutFloorApi extends GumnutLow { - private encodeGumnutData(lookupId: (inode: string, id: Id) => number, data: GumnutData): any { + private encodeGumnutData(lookupId: (inode: string, id: Id) => number, data: GumnutData) { switch (typeof data) { + case 'string': + return { s: data }; + case 'bigint': return { p: `b:${data.toString()}` }; @@ -56,9 +58,7 @@ class GumnutFloorApi extends GumnutLow { throw new Error(`unexpected data: ${JSON.stringify(data)}`); } - if (data instanceof GumnutInternString) { - return { s: data.s }; - } else if (data instanceof InoPosRefImpl) { + if (data instanceof InoPosRefImpl) { // we need to deref the ID here - on the way up to the server const pos = lookupId(data.target, data.id); return { s: data.target, t: pos }; @@ -110,7 +110,7 @@ class GumnutFloorApi extends GumnutLow { const id = resolvePos(raw.s, raw.t); return new InoPosRefImpl(raw.s, id); } - return internString(raw.s); + return raw.s; } throw new Error(`unexpected raw: ${JSON.stringify(raw)}`); diff --git a/client/lib/gumnut/intern.test.ts b/client/lib/gumnut/intern.test.ts deleted file mode 100644 index a8baea2..0000000 --- a/client/lib/gumnut/intern.test.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { test } from 'node:test'; -import * as assert from 'node:assert'; -import { internString } from './intern.ts'; - -test('intern', () => { - assert.strictEqual(internString('a'), internString('a')); - assert.notStrictEqual(internString('a'), internString('b')); -}); diff --git a/client/lib/gumnut/intern.ts b/client/lib/gumnut/intern.ts deleted file mode 100644 index b7af964..0000000 --- a/client/lib/gumnut/intern.ts +++ /dev/null @@ -1,42 +0,0 @@ -/** - * Contains an intered string for Gumnut. - * - * These instances can always be compared by reference. - */ -export class GumnutInternString { - constructor( - secret: Map, - public readonly s: string, - ) { - if (secret !== stringMap) { - throw new Error(`can't create GumnuutInternString`); - } - } - - toString() { - return this.s; - } -} - -const stringMap = new Map>(); - -const fr = new FinalizationRegistry((s: string) => stringMap.delete(s)); - -/** - * Creates a comparable interned string for Gumnut. - * - * This string cannot be collaborated on, it's purely a single-item entry. - */ -export function internString(s: string) { - const prev = stringMap.get(s); - const intern = prev?.deref(); - if (intern) { - return intern; - } - - const anew = new GumnutInternString(stringMap, s); - stringMap.set(s, new WeakRef(anew)); - fr.register(anew, s); - - return anew; -} diff --git a/client/lib/gumnut/types.ts b/client/lib/gumnut/types.ts index 9840abe..98e1abf 100644 --- a/client/lib/gumnut/types.ts +++ b/client/lib/gumnut/types.ts @@ -1,5 +1,4 @@ import type { Id } from '../ymodel/internal/shared.ts'; -import type { GumnutInternString } from './intern.ts'; /** * Gumnut supports these types, plus JS strings. @@ -10,7 +9,7 @@ export type GumnutData = | boolean | number | bigint - | GumnutInternString // intern string + | string // intern string | InoPosRef | typeof emptySymbol; // explicit void, when op is run without set diff --git a/client/lib/ymodel/api.test.ts b/client/lib/ymodel/api.test.ts index ba072fc..6b98da5 100644 --- a/client/lib/ymodel/api.test.ts +++ b/client/lib/ymodel/api.test.ts @@ -43,7 +43,7 @@ test('api', () => { api.applyOp({ r: [0], length: 7 }); const id = api.posToId(4); - const xxSet = api.applySet({ before: api.posOf(id), body: ['XX'] }); + const xxSet = api.applySet({ before: api.posOf(id), body: 'XX' }); assert.strictEqual(api.length(), 7); assert.deepStrictEqual(api.readString(), '\ufffd\ufffdXX\ufffd\ufffd\ufffd'); @@ -54,7 +54,7 @@ test('api', () => { yield ['abc']; xxSet.rollback(); - api.applySet({ before: api.posOf(id), body: ['YY'] }); + api.applySet({ before: api.posOf(id), body: 'YY' }); ++calls; }); @@ -84,13 +84,13 @@ test('prepareForServer', () => { l.perform(function* (byIno) { const api = byIno('abc'); api.applyOp({ r: [0], length: 2 }); - api.applySet({ before: 2, body: ['XX'] }); + api.applySet({ before: 2, body: 'XX' }); }); l.perform(function* (byIno) { const api = byIno('abc'); api.applyOp({ r: [1], length: 1 }); - api.applySet({ before: 2, body: ['x'] }); + api.applySet({ before: 2, body: 'x' }); }); const outer = l.byIno('abc'); @@ -106,7 +106,7 @@ test('prepareForServer', () => { update: { abc: { run: [{ r: [0], length: 2 }], - set: [{ before: -1, body: ['XX'] }], + set: [{ before: -1, body: 'XX' }], }, }, }, @@ -118,7 +118,7 @@ test('prepareForServer', () => { update: { abc: { run: [{ r: [-2], length: 1 }], - set: [{ before: -1, body: ['x'] }], + set: [{ before: -1, body: 'x' }], }, }, }, diff --git a/client/lib/ymodel/internal/state.test.ts b/client/lib/ymodel/internal/state.test.ts index 9ec8ed0..80071f7 100644 --- a/client/lib/ymodel/internal/state.test.ts +++ b/client/lib/ymodel/internal/state.test.ts @@ -6,7 +6,8 @@ test('state', () => { const r = RopeState.new(0); r.applyOp(r.coerceOp({ r: [0], length: 5 })); - r.applySet(r.coerceSet({ before: 5, body: ['abc', 1, 2] })); + r.applySet(r.coerceSet({ before: 5, body: [100, 1, 2] })); + r.applySet(r.coerceSet({ before: 3, body: 'abc' })); assert.deepStrictEqual(r.readString(), 'abc\ufffd\ufffd'); assert.deepStrictEqual(r.readData(), [0, 0, 0, 1, 2]); diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index 559adea..f812db2 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -22,7 +22,6 @@ function lengthOfEntry(e: Entry): number { /** * Internal state. - * {@link X} cannot be `string`. */ export class RopeState { private readonly rope: Rope>; @@ -79,7 +78,7 @@ export class RopeState { let length: number; switch (typeof d) { case 'number': - length = d; + length = d; // unset data break; case 'string': length = d.length; // TODO: disambiguate string data - e.g. "stringZero" symbol? @@ -87,11 +86,7 @@ export class RopeState { default: return d; } - const out: X[] = []; - for (let i = 0; i < length; ++i) { - out.push(this.zeroValue); - } - return out; + return Array(length).fill(this.zeroValue); }; const out: X[] = []; @@ -290,38 +285,17 @@ export class RopeState { } public coerceSet(set: SetPart): InternalSet { - let length = 0; - - for (const each of set.body) { - if (typeof each === 'string') { - length += each.length; - } else { - ++length; - } - } + const length = set.body.length; const start = set.before - length; - let at = start; - // TODO: right now, InteralSet splays out every ID/data entry - individual characters! - const out: InternalSet = { ids: [], data: [] }; - - for (const each of set.body) { - if (typeof each !== 'string') { - // simple item case - ++at; - const id = this.posToId(at); - out.ids.push(id); - out.data.push(each); - continue; - } + // TODO: right now, InteralSet splays out every entry or char with a target ID! + const out: InternalSet = { ids: [], data: set.body }; - // string case - for (let i = 0; i < each.length; ++i) { - ++at; - const id = this.posToId(at); - out.ids.push(id); - out.data.push(each.slice(i, i + 1)); - } + let at = start; + for (let i = 0; i < length; ++i) { + ++at; + const id = this.posToId(at); + out.ids.push(id); } return out; @@ -348,27 +322,19 @@ export class RopeState { } }; - let idx = 0; - - // TODO: this is horrible - we create a 1-length entry for each bit - for (const each of int.data) { - if (typeof each !== 'string') { - const id = int.ids[idx++]; - setAtId(id, [each]); - continue; - } - - for (let i = 0; i < each.length; ++i) { - const id = int.ids[idx++]; - setAtId(id, each.slice(i, i + 1)); - } - } + // TODO: this is horrible - we create a 1-length entry for each item + // TODO: work out if we can determine contiguous block ... + int.ids.forEach((id, index) => { + setAtId(id, int.data.slice(index, index + 1)); + }); return change; } /** * Finds the {@link Id} directly to the left of the one passed. + * + * This includes now-deleted IDs. */ private leftOf(id: Id): Id { if (id === zeroId) { @@ -395,7 +361,7 @@ export class RopeState { export type InternalOp = { op: Op; id: Id }; -export type InternalSet = { ids: Id[]; data: (string | X)[] }; +export type InternalSet = { ids: Id[]; data: X[] | string }; export type InternalPatch = { run: InternalOp[]; diff --git a/client/lib/ymodel/types.ts b/client/lib/ymodel/types.ts index af36083..0a56484 100644 --- a/client/lib/ymodel/types.ts +++ b/client/lib/ymodel/types.ts @@ -46,40 +46,47 @@ export function decodeOpRun(run: number[]): Op[] { export type SetPart = { before: number; - body: (X | string)[]; + body: X[] | string; }; -export function encodeSetOp(parts: SetPart[], encode: (x: X) => any = (x) => x): any[] { +export type NotString = T extends string ? never : T; + +/** + * Encodes this set op. + */ +export function encodeSetOp(parts: SetPart[], encode: (x: X) => NotString): any[] { const out: any[] = []; for (const part of parts) { - out.push( - part.before, - part.body.map((p) => { - if (typeof p === 'string') { - return p; - } - return encode(p); - }), - ); + if (typeof part.body === 'string') { + out.push(part.before, part.body); + } else { + out.push( + part.before, + part.body.map((p) => encode(p)), + ); + } } return out; } +/** + * Decodes this set op. + */ export function decodeSetOp(parts: any[], decode: (raw: any) => X = (x) => x): SetPart[] { const out: SetPart[] = []; if (parts.length % 2 !== 0) { throw new Error('model encoding error'); } for (let i = 0; i < parts.length; i += 2) { - out.push({ - before: parts[i] as number, - body: (parts[i + 1] as any[]).map((p) => { - if (typeof p === 'string') { - return p; - } - return decode(p); - }), - }); + const before = parts[i] as number; + const raw = parts[i + 1]; + + if (typeof raw === 'string') { + out.push({ before, body: raw }); + } else { + const body = (raw as any[]).map((p) => decode(p)); + out.push({ before, body }); + } } return out; } @@ -99,7 +106,7 @@ export type WirePatch = { s?: any[]; }; -export function encodePatch(patch: Patch, encode: (x: X) => any = (x) => x): WirePatch { +export function encodePatch(patch: Patch, encode: (x: X) => NotString): WirePatch { const run = encodeOpRun(patch.run); const set = encodeSetOp(patch.set, encode); const out: WirePatch = {}; diff --git a/client/lib/ymodel/work-holder.test.ts b/client/lib/ymodel/work-holder.test.ts index d85b7d6..3ef5ac1 100644 --- a/client/lib/ymodel/work-holder.test.ts +++ b/client/lib/ymodel/work-holder.test.ts @@ -22,7 +22,7 @@ test('generic', () => { assert.deepStrictEqual(w.readData(), [1, 2, 3, 0, 0]); // now include some string data - w.applySet({ before: 4, body: ['HI'] }); + w.applySet({ before: 4, body: 'HI' }); assert.deepStrictEqual(w.readData(), [1, 2, 0, 0, 0]); assert.deepStrictEqual(w.readString(), '\ufffd\ufffdHI\ufffd'); @@ -36,9 +36,13 @@ test('generic', () => { }, ]); assert.deepStrictEqual(out?.set, [ + { + before: -4, + body: [1, 2], + }, { before: -2, - body: [1, 2, 'HI'], + body: 'HI', }, ]); @@ -59,9 +63,13 @@ test('generic', () => { }, ]); assert.deepStrictEqual(out?.set, [ + { + before: -4, + body: [1, 2], + }, { before: -3, - body: [1, 2, 'H'], + body: 'H', }, ]); @@ -78,9 +86,13 @@ test('generic', () => { }, ]); assert.deepStrictEqual(out?.set, [ + { + before: -4, + body: [1, 2], + }, { before: -2, - body: [1, 2, 'HI'], + body: 'HI', }, ]); }); diff --git a/client/lib/ymodel/work-holder.ts b/client/lib/ymodel/work-holder.ts index fd0f790..0b6be76 100644 --- a/client/lib/ymodel/work-holder.ts +++ b/client/lib/ymodel/work-holder.ts @@ -132,35 +132,18 @@ export class WorkHolder implements PatchApi { } // step #2: convert step to real + // for now, this creates many 1-length (!) operations that are later merged for (const set of this.patch.set) { - let idx = 0; - - for (const each of set.data) { - if (typeof each !== 'string') { - const id = set.ids[idx++]; - if (m.posOfValid(id) === -1) { - continue; // don't send now-deleted sets - } - - out.set.push({ - before: combinedLookupId(length, id), - body: [each], - }); - continue; + set.ids.forEach((id, index) => { + if (m.posOfValid(id) === -1) { + return; // don't send now-deleted sets } - for (let i = 0; i < each.length; ++i) { - const id = set.ids[idx++]; - if (m.posOfValid(id) === -1) { - continue; // don't send now-deleted sets - } - - out.set.push({ - before: combinedLookupId(length, id), - body: [each.slice(i, i + 1)], - }); - } - } + out.set.push({ + before: combinedLookupId(length, id), + body: set.data.slice(index, index + 1), + }); + }); } out.set = mergeSetPart(out.set); @@ -287,6 +270,7 @@ function mergeSetPart(all: SetPart[]): SetPart[] { } unique.set(part.before, part); } + const sorted = Array.from(unique.values()); sorted.sort((a, b) => a.before - b.before); @@ -300,20 +284,22 @@ function mergeSetPart(all: SetPart[]): SetPart[] { const prev = out[out.length - 2]; const adjacent = part.before - part.body.length === prev.before; - if (!adjacent || (prev.before < 0 && part.before >= 0)) { - continue; // can't run over zero + if ( + !adjacent || + (prev.before < 0 && part.before >= 0) || + typeof prev.body !== typeof part.body + ) { + // not adjacent, can't run over zero, or not same type (string/array) + continue; } - out.pop(); - prev.before = part.before; - - if (typeof part.body[0] === 'string') { - if (typeof prev.body.at(-1) === 'string') { - prev.body[prev.body.length - 1] += part.body[0]; - continue; - } + out.pop(); + if (typeof part.body === 'string') { + (prev.body as string) += part.body; + } else { + (prev.body as X[]).push(...part.body); } - prev.body.push(...part.body); // should only be [0] + prev.before = part.before; } return out; diff --git a/tests/integration/basic.test.ts b/tests/integration/basic.test.ts index e584f9b..c059b16 100644 --- a/tests/integration/basic.test.ts +++ b/tests/integration/basic.test.ts @@ -2,8 +2,7 @@ import { test } from 'node:test'; import * as assert from 'node:assert'; import { connectToGumnutLow } from './util.ts'; import { checkCond } from 'thorish'; -import { type GumnutData, InoPosRef } from '../../client/lib/gumnut/types.ts'; -import { internString } from '../../client/lib/gumnut/intern.ts'; +import { InoPosRef } from '../../client/lib/gumnut/types.ts'; test('basic', async (t) => { const doc1 = await connectToGumnutLow(t.signal); @@ -20,7 +19,7 @@ test('basic', async (t) => { doc2.floor.perform(function* (t) { const api = t.byIno('abc'); - api.set(5, internString('interned string'), -100n, -Infinity, undefined); + api.set(5, 'interned string', -100n, -Infinity, undefined); }); await checkCond(() => { @@ -29,7 +28,7 @@ test('basic', async (t) => { const check = ino.data(); assert.deepStrictEqual(check, [ null, - internString('interned string'), + 'interned string', -100n, -Infinity, undefined, From 6e4d9082ac336ea4bc9085d96f88a6898036b301 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Wed, 18 Mar 2026 14:21:40 +1100 Subject: [PATCH 4/5] slight optimize state --- client/lib/ymodel/internal/state.test.ts | 6 ++ client/lib/ymodel/internal/state.ts | 83 +++++++++++++++++------- 2 files changed, 67 insertions(+), 22 deletions(-) diff --git a/client/lib/ymodel/internal/state.test.ts b/client/lib/ymodel/internal/state.test.ts index 80071f7..48546b5 100644 --- a/client/lib/ymodel/internal/state.test.ts +++ b/client/lib/ymodel/internal/state.test.ts @@ -7,8 +7,14 @@ test('state', () => { r.applyOp(r.coerceOp({ r: [0], length: 5 })); r.applySet(r.coerceSet({ before: 5, body: [100, 1, 2] })); + assert.deepStrictEqual(r.readData(), [0, 0, 100, 1, 2]); + r.applySet(r.coerceSet({ before: 3, body: 'abc' })); + assert.strictEqual(r.length(), 5); assert.deepStrictEqual(r.readString(), 'abc\ufffd\ufffd'); assert.deepStrictEqual(r.readData(), [0, 0, 0, 1, 2]); + + r.applySet(r.coerceSet({ before: 4, body: [999] })); + assert.deepStrictEqual(r.readData(), [0, 0, 0, 999, 2]); }); diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index f812db2..3d2e4d7 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -10,10 +10,12 @@ function splitEntryFromEnd(fromEnd: number, e: Entry): [Entry, Entry throw new Error(`split out of range`); } + const fromStart = length - fromEnd; + if (typeof e === 'number') { - return [length - fromEnd, fromEnd]; + return [fromStart, fromEnd]; } - return [e.slice(0, length - fromEnd), e.slice(fromEnd)]; + return [e.slice(0, fromStart), e.slice(fromStart)]; } function lengthOfEntry(e: Entry): number { @@ -302,33 +304,70 @@ export class RopeState { } public applySet(int: InternalSet): boolean { - let change = false; + if (int.ids.length === 0) { + return false; + } - const setAtId = (id: Id, data: X[] | string) => { - const prevId = this.leftOf(id); - this.ensureIdEdge(id); - this.ensureIdEdge(prevId); - const info = this.rope.lookup(id); + let i = 0; + + // fast-path array inserts + if (typeof int.data !== 'string') { + while (i < int.ids.length) { + let id = int.ids[i]; - if (info.length) { - if (data.length !== 1 || info.length !== 1) { - throw new Error(`panic: 1-index for now`); + const hostId = this.tree.equalAfter(id); + if (hostId === undefined) { + throw new Error(`panic; bad ID`); } - // if not already deleted, replace data - this.rope.adjust(id, data, 1); - info.data = data; - change = true; + const e = this.rope.lookup(hostId); + if (!Array.isArray(e.data)) { + break; // give up + } + + const index = e.length - 1 - (hostId - id); + e.data[index] = int.data[i]; + ++i; } - }; - // TODO: this is horrible - we create a 1-length entry for each item - // TODO: work out if we can determine contiguous block ... - int.ids.forEach((id, index) => { - setAtId(id, int.data.slice(index, index + 1)); - }); + if (i === int.ids.length) { + return true; + } + } + + // consume as much contiguous data and set, rinse and repeat + let lastId = this.leftOf(int.ids[0]); + this.ensureIdEdge(lastId); + + while (i < int.ids.length) { + let id = int.ids[i]; + const lo = i; + + while (++i < int.ids.length) { + const nextId = int.ids[i]; + if (this.leftOf(nextId) !== id) { + break; + } + id = nextId; + } + + const length = i - lo; + const data = int.data.slice(lo, i); + + this.ensureIdEdge(id); + const info = this.rope.lookup(id); + + if (info.length === length) { + this.rope.adjust(id, data, length); + } else { + this.rope.deleteTo(lastId, id); + this.rope.insertAfter(lastId, id, length, data); + } + + lastId = id; + } - return change; + return true; } /** From 06989add6e7f0562bab6fa144b33e425c0234120 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Wed, 18 Mar 2026 14:31:30 +1100 Subject: [PATCH 5/5] cleanup state --- client/lib/ymodel/internal/state.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index 3d2e4d7..6b2dac2 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -360,6 +360,15 @@ export class RopeState { if (info.length === length) { this.rope.adjust(id, data, length); } else { + // remove all tree entries >lastId