From 8ed880963fbf40fa35071026674dda0387059997 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Fri, 6 Jun 2025 00:59:06 +0200 Subject: [PATCH] work on allowing skips in struct store --- src/structs/AbstractStruct.js | 7 +++++ src/structs/GC.js | 14 +++++++-- src/structs/Item.js | 17 ++++++++--- src/structs/Skip.js | 32 +++++++++++++++----- src/utils/IdSet.js | 22 ++++++++------ src/utils/StructSet.js | 2 +- src/utils/StructStore.js | 23 +++++++++++--- src/utils/Transaction.js | 3 +- src/utils/encoding.js | 57 +++++++++++++++++------------------ tests/testHelper.js | 6 ---- tests/updates.tests.js | 8 ++--- tests/y-array.tests.js | 2 +- 12 files changed, 123 insertions(+), 70 deletions(-) diff --git a/src/structs/AbstractStruct.js b/src/structs/AbstractStruct.js index 52773eb7..c0889f8e 100644 --- a/src/structs/AbstractStruct.js +++ b/src/structs/AbstractStruct.js @@ -48,4 +48,11 @@ export class AbstractStruct { integrate (transaction, offset) { throw error.methodUnimplemented() } + + /** + * @param {number} diff + */ + splice (diff) { + throw error.methodUnimplemented() + } } diff --git a/src/structs/GC.js b/src/structs/GC.js index 710751cb..788cb5ec 100644 --- a/src/structs/GC.js +++ b/src/structs/GC.js @@ -3,7 +3,8 @@ import { addStruct, addStructToIdSet, addToIdSet, - UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction // eslint-disable-line + UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction, // eslint-disable-line + createID } from '../internals.js' export const structGCRefNumber = 0 @@ -32,7 +33,7 @@ export class GC extends AbstractStruct { /** * @param {Transaction} transaction - * @param {number} offset + * @param {number} offset - @todo remove offset parameter */ integrate (transaction, offset) { if (offset > 0) { @@ -61,4 +62,13 @@ export class GC extends AbstractStruct { getMissing (transaction, store) { return null } + + /** + * @param {number} diff + */ + splice (diff) { + const other = new GC(createID(this.id.client, this.id.clock + diff), this.length - diff) + this.length = diff + return other + } } diff --git a/src/structs/Item.js b/src/structs/Item.js index d6cbfa4d..c0b281ec 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -22,6 +22,7 @@ import { readContentType, addChangedTypeToTransaction, addStructToIdSet, + Skip, IdSet, StackItem, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction, // eslint-disable-line } from '../internals.js' @@ -117,6 +118,9 @@ export const splitItem = (transaction, leftItem, diff) => { if (rightItem.parentSub !== null && rightItem.right === null) { /** @type {AbstractType} */ (rightItem.parent)._map.set(rightItem.parentSub, rightItem) } + } else { + rightItem.left = null + rightItem.right = null } leftItem.length = diff return rightItem @@ -368,18 +372,16 @@ export class Item extends AbstractStruct { * @return {null | number} */ getMissing (transaction, store) { - if (this.origin && this.origin.client !== this.id.client && this.origin.clock >= getState(store, this.origin.client)) { + if (this.origin && (this.origin.clock >= getState(store, this.origin.client) || store.skips.hasId(this.origin))) { return this.origin.client } - if (this.rightOrigin && this.rightOrigin.client !== this.id.client && this.rightOrigin.clock >= getState(store, this.rightOrigin.client)) { + if (this.rightOrigin && (this.rightOrigin.clock >= getState(store, this.rightOrigin.client) || store.skips.hasId(this.rightOrigin))) { return this.rightOrigin.client } - if (this.parent && this.parent.constructor === ID && this.id.client !== this.parent.client && this.parent.clock >= getState(store, this.parent.client)) { + if (this.parent && this.parent.constructor === ID && (this.parent.clock >= getState(store, this.parent.client) || store.skips.hasId(this.parent))) { return this.parent.client } - // We have all missing ids, now find the items - if (this.origin) { this.left = getItemCleanEnd(transaction, store, this.origin) this.origin = this.left.lastId @@ -407,6 +409,11 @@ export class Item extends AbstractStruct { this.parent = /** @type {ContentType} */ (parentItem.content).type } } + // @todo remove thgis + if (this.left instanceof Skip || this.right instanceof Skip || this.parent instanceof Skip) { + debugger + throw new Error('dtruinae') + } return null } diff --git a/src/structs/Skip.js b/src/structs/Skip.js index 3f7caafa..1fa81a58 100644 --- a/src/structs/Skip.js +++ b/src/structs/Skip.js @@ -1,8 +1,11 @@ import { AbstractStruct, - UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction, ID // eslint-disable-line + addStruct, + addToIdSet, + UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction, // eslint-disable-line + createID } from '../internals.js' -import * as error from 'lib0/error' + import * as encoding from 'lib0/encoding' export const structSkipRefNumber = 10 @@ -12,7 +15,7 @@ export const structSkipRefNumber = 10 */ export class Skip extends AbstractStruct { get deleted () { - return true + return false } delete () {} @@ -34,8 +37,12 @@ export class Skip extends AbstractStruct { * @param {number} offset */ integrate (transaction, offset) { - // skip structs cannot be integrated - error.unexpectedCase() + if (offset > 0) { + this.id.clock += offset + this.length -= offset + } + addToIdSet(transaction.doc.store.skips, this.id.client, this.id.clock, this.length) + addStruct(transaction.doc.store, this) } /** @@ -49,11 +56,20 @@ export class Skip extends AbstractStruct { } /** - * @param {Transaction} transaction - * @param {StructStore} store + * @param {Transaction} _transaction + * @param {StructStore} _store * @return {null | number} */ - getMissing (transaction, store) { + getMissing (_transaction, _store) { return null } + + /** + * @param {number} diff + */ + splice (diff) { + const other = new Skip(createID(this.id.client, this.id.clock + diff), this.length - diff) + this.length = diff + return other + } } diff --git a/src/utils/IdSet.js b/src/utils/IdSet.js index 1f60eba4..a9e6644d 100644 --- a/src/utils/IdSet.js +++ b/src/utils/IdSet.js @@ -7,7 +7,7 @@ import { IdMap, AttrRanges, AttrRange, - AbstractStruct, DSDecoderV1, IdSetEncoderV1, DSDecoderV2, IdSetEncoderV2, Item, GC, StructStore, Transaction, ID, AttributionItem, // eslint-disable-line + Skip, AbstractStruct, DSDecoderV1, IdSetEncoderV1, DSDecoderV2, IdSetEncoderV2, Item, GC, StructStore, Transaction, ID, AttributionItem, // eslint-disable-line } from '../internals.js' import * as array from 'lib0/array' @@ -747,24 +747,28 @@ export const readAndApplyDeleteSet = (decoder, transaction, store) => { let index = findIndexSS(structs, clock) /** * We can ignore the case of GC and Delete structs, because we are going to skip them - * @type {Item} + * @type {Item | GC | Skip} */ - // @ts-ignore let struct = structs[index] // split the first item if necessary - if (!struct.deleted && struct.id.clock < clock) { - structs.splice(index + 1, 0, splitItem(transaction, struct, clock - struct.id.clock)) - index++ // increase we now want to use the next struct + if (!struct.deleted && struct.id.clock < clock && struct instanceof Item) { + // increment index, we now want to use the next struct + structs.splice(++index, 0, splitItem(transaction, struct, clock - struct.id.clock)) } while (index < structs.length) { // @ts-ignore struct = structs[index++] if (struct.id.clock < clockEnd) { if (!struct.deleted) { - if (clockEnd < struct.id.clock + struct.length) { - structs.splice(index, 0, splitItem(transaction, struct, clockEnd - struct.id.clock)) + if (struct instanceof Item) { + if (clockEnd < struct.id.clock + struct.length) { + structs.splice(index, 0, splitItem(transaction, struct, clockEnd - struct.id.clock)) + } + struct.delete(transaction) + } else { // is a Skip - add range to unappliedDS + const c = math.max(struct.id.clock, clock) + unappliedDS.add(client, c, math.min(struct.length, clockEnd - c)) } - struct.delete(transaction) } } else { break diff --git a/src/utils/StructSet.js b/src/utils/StructSet.js index 779068ad..3d60a0c8 100644 --- a/src/utils/StructSet.js +++ b/src/utils/StructSet.js @@ -105,7 +105,7 @@ export const removeRangesFromStructSet = (ss, exclude) => { structs[startIndex] = new Skip(new ID(client, range.clock), range.len) const d = endIndex - startIndex if (d > 1) { - structs.splice(startIndex, d) + structs.splice(startIndex + 1, d - 1) } } } diff --git a/src/utils/StructStore.js b/src/utils/StructStore.js index 5ca7ba82..e2b3eca2 100644 --- a/src/utils/StructStore.js +++ b/src/utils/StructStore.js @@ -3,7 +3,9 @@ import { splitItem, createDeleteSetFromStructStore, createIdSet, - Transaction, ID, Item // eslint-disable-line + Transaction, ID, Item, // eslint-disable-line + Skip, + createID } from '../internals.js' import * as math from 'lib0/math' @@ -104,7 +106,20 @@ export const addStruct = (store, struct) => { } else { const lastStruct = structs[structs.length - 1] if (lastStruct.id.clock + lastStruct.length !== struct.id.clock) { - throw error.unexpectedCase() + // this replaces an integrated skip + let index = findIndexSS(structs, struct.id.clock) + const skip = structs[index] + const diffStart = struct.id.clock - skip.id.clock + const diffEnd = skip.id.clock + skip.length - struct.id.clock - struct.length + if (diffStart > 0) { + structs.splice(index++, 0, new Skip(createID(struct.id.client, struct.id.clock), diffStart)) + } + if (diffEnd > 0) { + structs.splice(index + 1, 0, new Skip(createID(struct.id.client, struct.id.clock + struct.length), diffEnd)) + } + structs[index] = struct + store.skips.delete(struct.id.client, struct.id.clock, struct.length) + return } } structs.push(struct) @@ -183,8 +198,8 @@ export const getItem = /** @type {function(StructStore,ID):Item} */ (find) export const findIndexCleanStart = (transaction, structs, clock) => { const index = findIndexSS(structs, clock) const struct = structs[index] - if (struct.id.clock < clock && struct instanceof Item) { - structs.splice(index + 1, 0, splitItem(transaction, struct, clock - struct.id.clock)) + if (struct.id.clock < clock) { + structs.splice(index + 1, 0, struct instanceof Item ? splitItem(transaction, struct, clock - struct.id.clock) : struct.splice(clock - struct.id.clock)) return index + 1 } return index diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 6f1acf38..eab66829 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -10,7 +10,8 @@ import { generateNewClientId, createID, cleanupYTextAfterTransaction, - IdSet, UpdateEncoderV1, UpdateEncoderV2, GC, StructStore, AbstractType, AbstractStruct, YEvent, Doc, // eslint-disable-line + IdSet, UpdateEncoderV1, UpdateEncoderV2, GC, StructStore, AbstractType, AbstractStruct, YEvent, Doc, + diffIdSet, // eslint-disable-line // insertIntoIdSet } from '../internals.js' diff --git a/src/utils/encoding.js b/src/utils/encoding.js index e4339e7c..163cafab 100644 --- a/src/utils/encoding.js +++ b/src/utils/encoding.js @@ -36,7 +36,8 @@ import { readStructSet, removeRangesFromStructSet, createIdSet, - StructSet, IdSet, DSDecoderV2, Doc, Transaction, GC, Item, StructStore // eslint-disable-line + StructSet, IdSet, DSDecoderV2, Doc, Transaction, GC, Item, StructStore, // eslint-disable-line + createID } from '../internals.js' import * as encoding from 'lib0/encoding' @@ -231,34 +232,31 @@ const integrateStructs = (transaction, store, clientsStructRefs) => { if (stackHead.constructor !== Skip) { const localClock = map.setIfUndefined(state, stackHead.id.client, () => getState(store, stackHead.id.client)) const offset = localClock - stackHead.id.clock - if (offset < 0) { - // update from the same client is missing + const missing = stackHead.getMissing(transaction, store) + if (missing !== null) { stack.push(stackHead) - updateMissingSv(stackHead.id.client, stackHead.id.clock - 1) - // hid a dead wall, add all items from stack to restSS - addStackToRestSS() - } else { - const missing = stackHead.getMissing(transaction, store) - if (missing !== null) { - stack.push(stackHead) - // get the struct reader that has the missing struct - /** - * @type {{ refs: Array, i: number }} - */ - const structRefs = clientsStructRefs.clients.get(/** @type {number} */ (missing)) || { refs: [], i: 0 } - if (structRefs.refs.length === structRefs.i) { - // This update message causally depends on another update message that doesn't exist yet - updateMissingSv(/** @type {number} */ (missing), getState(store, missing)) - addStackToRestSS() - } else { - stackHead = structRefs.refs[structRefs.i++] - continue - } - } else if (offset === 0 || offset < stackHead.length) { - // all fine, apply the stackhead - stackHead.integrate(transaction, offset) // since I'm splitting structs before integrating them, offset is no longer necessary - state.set(stackHead.id.client, stackHead.id.clock + stackHead.length) + // get the struct reader that has the missing struct + /** + * @type {{ refs: Array, i: number }} + */ + const structRefs = clientsStructRefs.clients.get(/** @type {number} */ (missing)) || { refs: [], i: 0 } + if (structRefs.refs.length === structRefs.i || missing === stackHead.id.client || stack.some(s => s.id.client === missing)) { // @todo this could be optimized! + // This update message causally depends on another update message that doesn't exist yet + updateMissingSv(/** @type {number} */ (missing), getState(store, missing)) + addStackToRestSS() + } else { + stackHead = structRefs.refs[structRefs.i++] + continue } + } else { + // all fine, apply the stackhead + // but first add a skip to structs if necessary + if (offset < 0) { + const skip = new Skip(createID(stackHead.id.client, localClock), -offset) + skip.integrate(transaction, 0) + } + stackHead.integrate(transaction, 0) + state.set(stackHead.id.client, math.max(stackHead.id.clock + stackHead.length, localClock)) } } // iterate to next stackHead @@ -321,7 +319,8 @@ export const readUpdateV2 = (decoder, ydoc, transactionOrigin, structDecoder = n ss.clients.forEach((_, client) => { const storeStructs = store.clients.get(client) if (storeStructs) { - knownState.add(client, 0, storeStructs.length) + const last = storeStructs[storeStructs.length - 1] + knownState.add(client, 0, last.id.clock + last.length) // remove known items from ss store.skips.clients.get(client)?.getIds().forEach(idrange => { knownState.delete(client, idrange.clock, idrange.len) @@ -339,7 +338,7 @@ export const readUpdateV2 = (decoder, ydoc, transactionOrigin, structDecoder = n if (pending) { // check if we can apply something for (const [client, clock] of pending.missing) { - if (clock < getState(store, client)) { + if (ss.clients.has(client) || clock < getState(store, client)) { retry = true break } diff --git a/tests/testHelper.js b/tests/testHelper.js index 37792c6d..3a9c456b 100644 --- a/tests/testHelper.js +++ b/tests/testHelper.js @@ -97,12 +97,6 @@ export class TestYInstance extends Y.Doc { } this.updates.push(update) }) - this.on('afterTransaction', tr => { - // @ts-ignore - if (Array.from(tr.insertSet.clients.values()).some(ids => ids._ids.length !== 1)) { - throw new Error('Currently, we expect that idset contains exactly one item per client.') - } - }) this.connect() } diff --git a/tests/updates.tests.js b/tests/updates.tests.js index 1cb84086..40ccc58a 100644 --- a/tests/updates.tests.js +++ b/tests/updates.tests.js @@ -169,19 +169,19 @@ const checkUpdateCases = (ydoc, updates, enc, hasDeletes) => { // t.info('Target State: ') // enc.logUpdate(targetState) - cases.forEach((mergedUpdates) => { - // t.info('State Case $' + i + ':') + cases.forEach((mergedUpdates, i) => { + t.info(`State Case $${i} (${enc.description}):`) // enc.logUpdate(updates) const merged = new Y.Doc({ gc: false }) enc.applyUpdate(merged, mergedUpdates) t.compareArrays(merged.getArray().toArray(), ydoc.getArray().toArray()) t.compare(enc.encodeStateVector(merged), enc.encodeStateVectorFromUpdate(mergedUpdates)) - if (enc.updateEventName !== 'update') { // @todo should this also work on legacy updates? for (let j = 1; j < updates.length; j++) { const partMerged = enc.mergeUpdates(updates.slice(j)) const partMeta = enc.parseUpdateMeta(partMerged) - const targetSV = Y.encodeStateVectorFromUpdateV2(Y.mergeUpdatesV2(updates.slice(0, j))) + + const targetSV = enc.encodeStateVectorFromUpdate(enc.mergeUpdates(updates.slice(0, j))) const diffed = enc.diffUpdate(mergedUpdates, targetSV) const diffedMeta = enc.parseUpdateMeta(diffed) t.compare(partMeta, diffedMeta) diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index 4eaab5d2..c2e9f7c7 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -604,7 +604,7 @@ const arrayTransactions = [ * @param {t.TestCase} tc */ export const testRepeatGeneratingYarrayTests6 = tc => { - applyRandomTests(tc, arrayTransactions, 6) + applyRandomTests(tc, arrayTransactions, 8) } /**