From 5293ab4df138b35ee94408013d592ac0e51063e4 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Tue, 9 Jun 2020 00:53:05 +0200 Subject: [PATCH] Improve memory usage by omitting the ItemRef step and directly applying the Item --- src/structs/AbstractStruct.js | 32 +-- src/structs/GC.js | 41 +--- src/structs/Item.js | 439 +++++++++++++++++----------------- src/types/AbstractType.js | 10 +- src/types/YText.js | 8 +- src/types/YXmlFragment.js | 2 +- src/utils/RelativePosition.js | 2 +- src/utils/StructStore.js | 11 +- src/utils/Transaction.js | 4 +- src/utils/YEvent.js | 4 +- src/utils/encoding.js | 61 +++-- src/utils/isParentOf.js | 2 +- tests/y-map.tests.js | 2 +- tests/y-text.tests.js | 32 ++- 14 files changed, 302 insertions(+), 348 deletions(-) diff --git a/src/structs/AbstractStruct.js b/src/structs/AbstractStruct.js index 21995ad1..06f83faa 100644 --- a/src/structs/AbstractStruct.js +++ b/src/structs/AbstractStruct.js @@ -39,39 +39,9 @@ export class AbstractStruct { /** * @param {Transaction} transaction - */ - integrate (transaction) { - throw error.methodUnimplemented() - } -} - -export class AbstractStructRef { - /** - * @param {ID} id - */ - constructor (id) { - this.id = id - /** - * @type {Array} - */ - this._missing = [] - } - - /** - * @param {Transaction} transaction - * @return {Array} - */ - getMissing (transaction) { - return this._missing - } - - /** - * @param {Transaction} transaction - * @param {StructStore} store * @param {number} offset - * @return {AbstractStruct} */ - toStruct (transaction, store, offset) { + integrate (transaction, offset) { throw error.methodUnimplemented() } } diff --git a/src/structs/GC.js b/src/structs/GC.js index b70b27a3..f2c5fcd6 100644 --- a/src/structs/GC.js +++ b/src/structs/GC.js @@ -1,12 +1,10 @@ import { - AbstractStructRef, AbstractStruct, addStruct, StructStore, Transaction, ID // eslint-disable-line } from '../internals.js' -import * as decoding from 'lib0/decoding.js' import * as encoding from 'lib0/encoding.js' export const structGCRefNumber = 0 @@ -37,8 +35,13 @@ export class GC extends AbstractStruct { /** * @param {Transaction} transaction + * @param {number} offset */ - integrate (transaction) { + integrate (transaction, offset) { + if (offset > 0) { + this.id.clock += offset + this.length -= offset + } addStruct(transaction.doc.store, this) } @@ -50,39 +53,13 @@ export class GC extends AbstractStruct { encoding.writeUint8(encoder, structGCRefNumber) encoding.writeVarUint(encoder, this.length - offset) } -} - -/** - * @private - */ -export class GCRef extends AbstractStructRef { - /** - * @param {decoding.Decoder} decoder - * @param {ID} id - * @param {number} info - */ - constructor (decoder, id, info) { - super(id) - /** - * @type {number} - */ - this.length = decoding.readVarUint(decoder) - } /** * @param {Transaction} transaction * @param {StructStore} store - * @param {number} offset - * @return {GC} + * @return {null | ID} */ - toStruct (transaction, store, offset) { - if (offset > 0) { - this.id.clock += offset - this.length -= offset - } - return new GC( - this.id, - this.length - ) + getMissing (transaction, store) { + return null } } diff --git a/src/structs/Item.js b/src/structs/Item.js index ceffeb51..96bae7bb 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -4,7 +4,6 @@ import { writeID, GC, getState, - AbstractStructRef, AbstractStruct, replaceStruct, addStruct, @@ -24,7 +23,7 @@ import { readContentFormat, readContentType, addChangedTypeToTransaction, - ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line + Doc, ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line } from '../internals.js' import * as error from 'lib0/error.js' @@ -73,7 +72,7 @@ export const followRedone = (store, id) => { export const keepItem = (item, keep) => { while (item !== null && item.keep !== keep) { item.keep = keep - item = item.parent._item + item = /** @type {AbstractType} */ (item.parent)._item } } @@ -119,7 +118,7 @@ export const splitItem = (transaction, leftItem, diff) => { transaction._mergeStructs.push(rightItem) // update parent._map if (rightItem.parentSub !== null && rightItem.right === null) { - rightItem.parent._map.set(rightItem.parentSub, rightItem) + /** @type {AbstractType} */ (rightItem.parent)._map.set(rightItem.parentSub, rightItem) } leftItem.length = diff return rightItem @@ -144,7 +143,7 @@ export const redoItem = (transaction, item, redoitems) => { if (redone !== null) { return getItemCleanStart(transaction, redone) } - let parentItem = item.parent._item + let parentItem = /** @type {AbstractType} */ (item.parent)._item /** * @type {Item|null} */ @@ -169,7 +168,7 @@ export const redoItem = (transaction, item, redoitems) => { } } if (left.right !== null) { - left = /** @type {Item} */ (item.parent._map.get(item.parentSub)) + left = /** @type {Item} */ (/** @type {AbstractType} */ (item.parent)._map.get(item.parentSub)) } right = null } @@ -191,10 +190,10 @@ export const redoItem = (transaction, item, redoitems) => { */ let leftTrace = left // trace redone until parent matches - while (leftTrace !== null && leftTrace.parent._item !== parentItem) { + while (leftTrace !== null && /** @type {AbstractType} */ (leftTrace.parent)._item !== parentItem) { leftTrace = leftTrace.redone === null ? null : getItemCleanStart(transaction, leftTrace.redone) } - if (leftTrace !== null && leftTrace.parent._item === parentItem) { + if (leftTrace !== null && /** @type {AbstractType} */ (leftTrace.parent)._item === parentItem) { left = leftTrace break } @@ -206,10 +205,10 @@ export const redoItem = (transaction, item, redoitems) => { */ let rightTrace = right // trace redone until parent matches - while (rightTrace !== null && rightTrace.parent._item !== parentItem) { + while (rightTrace !== null && /** @type {AbstractType} */ (rightTrace.parent)._item !== parentItem) { rightTrace = rightTrace.redone === null ? null : getItemCleanStart(transaction, rightTrace.redone) } - if (rightTrace !== null && rightTrace.parent._item === parentItem) { + if (rightTrace !== null && /** @type {AbstractType} */ (rightTrace.parent)._item === parentItem) { right = rightTrace break } @@ -228,7 +227,7 @@ export const redoItem = (transaction, item, redoitems) => { ) item.redone = nextId keepItem(redoneItem, true) - redoneItem.integrate(transaction) + redoneItem.integrate(transaction, 0) return redoneItem } @@ -242,7 +241,7 @@ export class Item extends AbstractStruct { * @param {ID | null} origin * @param {Item | null} right * @param {ID | null} rightOrigin - * @param {AbstractType} parent + * @param {AbstractType|ID|null} parent Is a type if integrated, is null if it is possible to copy parent from left or right, is ID before integration to search for it. * @param {string | null} parentSub * @param {AbstractContent} content */ @@ -251,7 +250,6 @@ export class Item extends AbstractStruct { /** * The item that was originally to the left of this item. * @type {ID | null} - * @readonly */ this.origin = origin /** @@ -266,14 +264,11 @@ export class Item extends AbstractStruct { this.right = right /** * The item that was originally to the right of this item. - * @readonly * @type {ID | null} */ this.rightOrigin = rightOrigin /** - * The parent type. - * @type {AbstractType} - * @readonly + * @type {AbstractType|ID|null} */ this.parent = parent /** @@ -282,7 +277,6 @@ export class Item extends AbstractStruct { * to insert this item. If `parentSub = null` type._start is the list in * which to insert to. Otherwise it is `parent._map`. * @type {String | null} - * @readonly */ this.parentSub = parentSub /** @@ -311,104 +305,178 @@ export class Item extends AbstractStruct { } /** + * Return missing ids, or define missing items and return null. + * * @param {Transaction} transaction + * @param {StructStore} store + * @return {null | ID} */ - integrate (transaction) { + getMissing (transaction, store) { + const origin = this.origin + const rightOrigin = this.rightOrigin + const parent = /** @type {ID} */ (this.parent) + + if (origin && origin.clock >= getState(store, origin.client)) { + return this.origin + } + if (rightOrigin && rightOrigin.clock >= getState(store, rightOrigin.client)) { + return this.rightOrigin + } + if (parent && parent.constructor === ID && parent.clock >= getState(store, parent.client)) { + return parent + } + + // We have all missing ids, now find the items + + if (origin) { + this.left = getItemCleanEnd(transaction, store, origin) + this.origin = this.left.lastId + } + if (rightOrigin) { + this.right = getItemCleanStart(transaction, rightOrigin) + this.rightOrigin = this.right.id + } + if (parent && parent.constructor === ID) { + if (parent.clock < getState(store, parent.client)) { + const parentItem = getItem(store, parent) + if (parentItem.constructor === GC) { + this.parent = null + } else { + this.parent = /** @type {ContentType} */ (parentItem.content).type + } + } else { + return parent + } + } + // only set item if this shouldn't be garbage collected + if (!this.parent) { + if (this.left && this.left.constructor === Item) { + this.parent = this.left.parent + this.parentSub = this.left.parentSub + } + if (this.right && this.right.constructor === Item) { + this.parent = this.right.parent + this.parentSub = this.right.parentSub + } + } + return null + } + + /** + * @param {Transaction} transaction + * @param {number} offset + */ + integrate (transaction, offset) { const store = transaction.doc.store - const parent = this.parent + if (offset > 0) { + this.id.clock += offset + this.left = getItemCleanEnd(transaction, store, createID(this.id.client, this.id.clock - 1)) + this.origin = this.left.lastId + this.content = this.content.splice(offset) + this.length -= offset + } const parentSub = this.parentSub const length = this.length - /** - * @type {Item|null} - */ - let left = this.left - /** - * @type {Item|null} - */ - let o - // set o to the first conflicting item - if (left !== null) { - o = left.right - } else if (parentSub !== null) { - o = parent._map.get(parentSub) || null - while (o !== null && o.left !== null) { - o = o.left - } - } else { - o = parent._start - } - // TODO: use something like DeleteSet here (a tree implementation would be best) - /** - * @type {Set} - */ - const conflictingItems = new Set() - /** - * @type {Set} - */ - const itemsBeforeOrigin = new Set() - // Let c in conflictingItems, b in itemsBeforeOrigin - // ***{origin}bbbb{this}{c,b}{c,b}{o}*** - // Note that conflictingItems is a subset of itemsBeforeOrigin - while (o !== null && o !== this.right) { - itemsBeforeOrigin.add(o) - conflictingItems.add(o) - if (compareIDs(this.origin, o.origin)) { - // case 1 - if (o.id.client < this.id.client) { - left = o - conflictingItems.clear() - } - } else if (o.origin !== null && itemsBeforeOrigin.has(getItem(store, o.origin))) { - // case 2 - if (o.origin === null || !conflictingItems.has(getItem(store, o.origin))) { - left = o - conflictingItems.clear() - } - } else { - break - } - o = o.right - } - this.left = left - // reconnect left/right + update parent map/start if necessary - if (left !== null) { - const right = left.right - this.right = right - left.right = this - } else { - let r - if (parentSub !== null) { - r = parent._map.get(parentSub) || null - while (r !== null && r.left !== null) { - r = r.left - } - } else { - r = parent._start - parent._start = this - } - this.right = r - } - if (this.right !== null) { - this.right.left = this - } else if (parentSub !== null) { - // set as current parent value if right === null and this is parentSub - parent._map.set(parentSub, this) + const parent = /** @type {AbstractType|null} */ (this.parent) + + if (parent) { + /** + * @type {Item|null} + */ + let left = this.left + + /** + * @type {Item|null} + */ + let o + // set o to the first conflicting item if (left !== null) { - // this is the current attribute value of parent. delete right - left.delete(transaction) + o = left.right + } else if (parentSub !== null) { + o = parent._map.get(parentSub) || null + while (o !== null && o.left !== null) { + o = o.left + } + } else { + o = parent._start } - } - // adjust length of parent - if (parentSub === null && this.countable && !this.deleted) { - parent._length += length - } - addStruct(store, this) - this.content.integrate(transaction, this) - // add parent to transaction.changed - addChangedTypeToTransaction(transaction, parent, parentSub) - if ((parent._item !== null && parent._item.deleted) || (this.right !== null && parentSub !== null)) { - // delete if parent is deleted or if this is not the current attribute value of parent - this.delete(transaction) + // TODO: use something like DeleteSet here (a tree implementation would be best) + // @todo use global set definitions + /** + * @type {Set} + */ + const conflictingItems = new Set() + /** + * @type {Set} + */ + const itemsBeforeOrigin = new Set() + // Let c in conflictingItems, b in itemsBeforeOrigin + // ***{origin}bbbb{this}{c,b}{c,b}{o}*** + // Note that conflictingItems is a subset of itemsBeforeOrigin + while (o !== null && o !== this.right) { + itemsBeforeOrigin.add(o) + conflictingItems.add(o) + if (compareIDs(this.origin, o.origin)) { + // case 1 + if (o.id.client < this.id.client) { + left = o + conflictingItems.clear() + } + } else if (o.origin !== null && itemsBeforeOrigin.has(getItem(store, o.origin))) { + // case 2 + if (o.origin === null || !conflictingItems.has(getItem(store, o.origin))) { + left = o + conflictingItems.clear() + } + } else { + break + } + o = o.right + } + this.left = left + // reconnect left/right + update parent map/start if necessary + if (left !== null) { + const right = left.right + this.right = right + left.right = this + } else { + let r + if (parentSub !== null) { + r = parent._map.get(parentSub) || null + while (r !== null && r.left !== null) { + r = r.left + } + } else { + r = parent._start + parent._start = this + } + this.right = r + } + if (this.right !== null) { + this.right.left = this + } else if (parentSub !== null) { + // set as current parent value if right === null and this is parentSub + parent._map.set(parentSub, this) + if (left !== null) { + // this is the current attribute value of parent. delete right + left.delete(transaction) + } + } + // adjust length of parent + if (parentSub === null && this.countable && !this.deleted) { + parent._length += length + } + addStruct(store, this) + this.content.integrate(transaction, this) + // add parent to transaction.changed + addChangedTypeToTransaction(transaction, parent, parentSub) + if ((parent._item !== null && parent._item.deleted) || (this.right !== null && parentSub !== null)) { + // delete if parent is deleted or if this is not the current attribute value of parent + this.delete(transaction) + } + } else { + // parent is not defined. Integrate GC struct instead + new GC(this.id, this.length).integrate(transaction, 0) } } @@ -481,7 +549,7 @@ export class Item extends AbstractStruct { */ delete (transaction) { if (!this.deleted) { - const parent = this.parent + const parent = /** @type {AbstractType} */ (this.parent) // adjust the length of parent if (this.countable && this.parentSub === null) { parent._length -= this.length @@ -534,7 +602,7 @@ export class Item extends AbstractStruct { writeID(encoder, rightOrigin) } if (origin === null && rightOrigin === null) { - const parent = this.parent + const parent = /** @type {AbstractType} */ (this.parent) const parentItem = parent._item if (parentItem === null) { // parent type on y._map @@ -670,122 +738,49 @@ export class AbstractContent { } /** - * @private + * @param {decoding.Decoder} decoder + * @param {ID} id + * @param {number} info + * @param {Doc} doc */ -export class ItemRef extends AbstractStructRef { +export const readItem = (decoder, id, info, doc) => { /** - * @param {decoding.Decoder} decoder - * @param {ID} id - * @param {number} info + * The item that was originally to the left of this item. + * @type {ID | null} */ - constructor (decoder, id, info) { - super(id) - /** - * The item that was originally to the left of this item. - * @type {ID | null} - */ - this.left = (info & binary.BIT8) === binary.BIT8 ? readID(decoder) : null - /** - * The item that was originally to the right of this item. - * @type {ID | null} - */ - this.right = (info & binary.BIT7) === binary.BIT7 ? readID(decoder) : null - const canCopyParentInfo = (info & (binary.BIT7 | binary.BIT8)) === 0 - const hasParentYKey = canCopyParentInfo ? decoding.readVarUint(decoder) === 1 : false - /** - * If parent = null and neither left nor right are defined, then we know that `parent` is child of `y` - * and we read the next string as parentYKey. - * It indicates how we store/retrieve parent from `y.share` - * @type {string|null} - */ - this.parentYKey = canCopyParentInfo && hasParentYKey ? decoding.readVarString(decoder) : null - /** - * The parent type. - * @type {ID | null} - */ - this.parent = canCopyParentInfo && !hasParentYKey ? readID(decoder) : null - /** - * If the parent refers to this item with some kind of key (e.g. YMap, the - * key is specified here. The key is then used to refer to the list in which - * to insert this item. If `parentSub = null` type._start is the list in - * which to insert to. Otherwise it is `parent._map`. - * @type {String | null} - */ - this.parentSub = canCopyParentInfo && (info & binary.BIT6) === binary.BIT6 ? decoding.readVarString(decoder) : null - const missing = this._missing - // Only add items to missing if they don't preceed this item (indicating that it has already been added). - // @todo Creating missing items could be done outside this constructor - if (this.left !== null && this.left.client !== id.client) { - missing.push(this.left) - } - if (this.right !== null && this.right.client !== id.client) { - missing.push(this.right) - } - if (this.parent !== null) { - missing.push(this.parent) - } - /** - * @type {AbstractContent} - */ - this.content = readItemContent(decoder, info) - this.length = this.content.getLength() - } + const origin = (info & binary.BIT8) === binary.BIT8 ? readID(decoder) : null + /** + * The item that was originally to the right of this item. + * @type {ID | null} + */ + const rightOrigin = (info & binary.BIT7) === binary.BIT7 ? readID(decoder) : null + const canCopyParentInfo = (info & (binary.BIT7 | binary.BIT8)) === 0 + const hasParentYKey = canCopyParentInfo ? decoding.readVarUint(decoder) === 1 : false + /** + * If parent = null and neither left nor right are defined, then we know that `parent` is child of `y` + * and we read the next string as parentYKey. + * It indicates how we store/retrieve parent from `y.share` + * @type {string|null} + */ + const parentYKey = canCopyParentInfo && hasParentYKey ? decoding.readVarString(decoder) : null + /** + * The parent type. + * @type {ID | AbstractType | null} + */ + const parent = canCopyParentInfo && !hasParentYKey ? readID(decoder) : (parentYKey ? doc.get(parentYKey) : null) + /** + * If the parent refers to this item with some kind of key (e.g. YMap, the + * key is specified here. The key is then used to refer to the list in which + * to insert this item. If `parentSub = null` type._start is the list in + * which to insert to. Otherwise it is `parent._map`. + * @type {String | null} + */ + const parentSub = canCopyParentInfo && (info & binary.BIT6) === binary.BIT6 ? decoding.readVarString(decoder) : null /** - * @param {Transaction} transaction - * @param {StructStore} store - * @param {number} offset - * @return {Item|GC} + * @type {AbstractContent} */ - toStruct (transaction, store, offset) { - if (offset > 0) { - this.id.clock += offset - this.left = createID(this.id.client, this.id.clock - 1) - this.content = this.content.splice(offset) - this.length -= offset - } + const content = readItemContent(decoder, info) - const left = this.left === null ? null : getItemCleanEnd(transaction, store, this.left) - const right = this.right === null ? null : getItemCleanStart(transaction, this.right) - const parentId = this.parent - let parent = null - let parentSub = this.parentSub - if (parentId !== null) { - const parentItem = getItem(store, parentId) - // Edge case: toStruct is called with an offset > 0. In this case left is defined. - // Depending in which order structs arrive, left may be GC'd and the parent not - // deleted. This is why we check if left is GC'd. Strictly we don't have - // to check if right is GC'd, but we will in case we run into future issues - if (!parentItem.deleted && (left === null || left.constructor !== GC) && (right === null || right.constructor !== GC)) { - parent = /** @type {ContentType} */ (parentItem.content).type - } - } else if (this.parentYKey !== null) { - parent = transaction.doc.get(this.parentYKey) - } else if (left !== null) { - if (left.constructor !== GC) { - parent = left.parent - parentSub = left.parentSub - } - } else if (right !== null) { - if (right.constructor !== GC) { - parent = right.parent - parentSub = right.parentSub - } - } else { - throw error.unexpectedCase() - } - - return parent === null - ? new GC(this.id, this.length) - : new Item( - this.id, - left, - left && left.lastId, - right, - right && right.id, - parent, - parentSub, - this.content - ) - } + return new Item(id, null, origin, null, rightOrigin, parent, parentSub, content) } diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index 8521d921..7f167edf 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -53,7 +53,7 @@ export const callTypeObservers = (type, transaction, event) => { if (type._item === null) { break } - type = type._item.parent + type = /** @type {AbstractType} */ (type._item.parent) } callEventHandlerListeners(changedType._eH, event, transaction) } @@ -386,7 +386,7 @@ export const typeListInsertGenericsAfter = (transaction, parent, referenceItem, const packJsonContent = () => { if (jsonContent.length > 0) { left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentAny(jsonContent)) - left.integrate(transaction) + left.integrate(transaction, 0) jsonContent = [] } } @@ -405,12 +405,12 @@ export const typeListInsertGenericsAfter = (transaction, parent, referenceItem, case Uint8Array: case ArrayBuffer: left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentBinary(new Uint8Array(/** @type {Uint8Array} */ (c)))) - left.integrate(transaction) + left.integrate(transaction, 0) break default: if (c instanceof AbstractType) { left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentType(c)) - left.integrate(transaction) + left.integrate(transaction, 0) } else { throw new Error('Unexpected content type in insert operation') } @@ -537,7 +537,7 @@ export const typeMapSet = (transaction, parent, key, value) => { } } } - new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, null, null, parent, key, content).integrate(transaction) + new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, null, null, parent, key, content).integrate(transaction, 0) } /** diff --git a/src/types/YText.js b/src/types/YText.js index a3f26778..81e87a0a 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -153,7 +153,7 @@ const insertNegatedAttributes = (transaction, parent, currPos, negatedAttributes const ownClientId = doc.clientID for (const [key, val] of negatedAttributes) { left = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentFormat(key, val)) - left.integrate(transaction) + left.integrate(transaction, 0) } currPos.left = left currPos.right = right @@ -228,7 +228,7 @@ const insertAttributes = (transaction, parent, currPos, currentAttributes, attri negatedAttributes.set(key, currentVal) const { left, right } = currPos currPos.left = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentFormat(key, val)) - currPos.left.integrate(transaction) + currPos.left.integrate(transaction, 0) } } return negatedAttributes @@ -259,7 +259,7 @@ const insertText = (transaction, parent, currPos, currentAttributes, text, attri const content = text.constructor === String ? new ContentString(/** @type {string} */ (text)) : new ContentEmbed(text) const { left, right } = currPos currPos.left = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, content) - currPos.left.integrate(transaction) + currPos.left.integrate(transaction, 0) return insertNegatedAttributes(transaction, parent, currPos, negatedAttributes) } @@ -320,7 +320,7 @@ const formatText = (transaction, parent, currPos, currentAttributes, length, att newlines += '\n' } left = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentString(newlines)) - left.integrate(transaction) + left.integrate(transaction, 0) } currPos.left = left currPos.right = right diff --git a/src/types/YXmlFragment.js b/src/types/YXmlFragment.js index 5048a925..36bdb2d9 100644 --- a/src/types/YXmlFragment.js +++ b/src/types/YXmlFragment.js @@ -97,7 +97,7 @@ export class YXmlTreeWalker { } else if (n.parent === this._root) { n = null } else { - n = n.parent._item + n = /** @type {AbstractType} */ (n.parent)._item } } } diff --git a/src/utils/RelativePosition.js b/src/utils/RelativePosition.js index 36b6e5b3..a4af4af7 100644 --- a/src/utils/RelativePosition.js +++ b/src/utils/RelativePosition.js @@ -227,7 +227,7 @@ export const createAbsolutePositionFromRelativePosition = (rpos, doc) => { if (!(right instanceof Item)) { return null } - type = right.parent + type = /** @type {AbstractType} */ (right.parent) if (type._item === null || !type._item.deleted) { index = right.deleted || !right.countable ? 0 : res.diff let n = right.left diff --git a/src/utils/StructStore.js b/src/utils/StructStore.js index d74bd787..01832a81 100644 --- a/src/utils/StructStore.js +++ b/src/utils/StructStore.js @@ -2,7 +2,7 @@ import { GC, splitItem, - AbstractStruct, GCRef, ItemRef, Transaction, ID, Item // eslint-disable-line + AbstractStruct, Transaction, ID, Item // eslint-disable-line } from '../internals.js' import * as math from 'lib0/math.js' @@ -21,13 +21,13 @@ export class StructStore { * We could shift the array of refs instead, but shift is incredible * slow in Chrome for arrays with more than 100k elements * @see tryResumePendingStructRefs - * @type {Map}>} + * @type {Map}>} */ this.pendingClientsStructRefs = new Map() /** * Stack of pending structs waiting for struct dependencies * Maximum length of stack is structReaders.size - * @type {Array} + * @type {Array} */ this.pendingStack = [] /** @@ -129,7 +129,10 @@ export const findIndexSS = (structs, clock) => { if (mid.id.clock === clock) { return right } - let midindex = math.floor((clock / (midclock + mid.length)) * right) // pivoting the search + // @todo does it even make sense to pivot the search? + // If a good split misses, it might actually increase the time to find the correct item. + // Currently, the only advantage is that search with pivoting might find the item on the first try. + let midindex = math.floor((clock / (midclock + mid.length - 1)) * right) // pivoting the search while (left <= right) { mid = structs[midindex] midclock = mid.id.clock diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index be4a4ca4..ea2919b6 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -156,8 +156,8 @@ const tryToMergeWithLeft = (structs, pos) => { if (left.deleted === right.deleted && left.constructor === right.constructor) { if (left.mergeWith(right)) { structs.splice(pos, 1) - if (right instanceof Item && right.parentSub !== null && right.parent._map.get(right.parentSub) === right) { - right.parent._map.set(right.parentSub, /** @type {Item} */ (left)) + if (right instanceof Item && right.parentSub !== null && /** @type {AbstractType} */ (right.parent)._map.get(right.parentSub) === right) { + /** @type {AbstractType} */ (right.parent)._map.set(right.parentSub, /** @type {Item} */ (left)) } } } diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index aa679de6..dd3cfbf2 100644 --- a/src/utils/YEvent.js +++ b/src/utils/YEvent.js @@ -211,7 +211,7 @@ const getPathTo = (parent, child) => { } else { // parent is array-ish let i = 0 - let c = child._item.parent._start + let c = /** @type {AbstractType} */ (child._item.parent)._start while (c !== child._item && c !== null) { if (!c.deleted) { i++ @@ -220,7 +220,7 @@ const getPathTo = (parent, child) => { } path.unshift(i) } - child = child._item.parent + child = /** @type {AbstractType} */ (child._item.parent) } return path } diff --git a/src/utils/encoding.js b/src/utils/encoding.js index f85a5a00..e7d4aeba 100644 --- a/src/utils/encoding.js +++ b/src/utils/encoding.js @@ -16,8 +16,6 @@ import { findIndexSS, - GCRef, - ItemRef, writeID, readID, getState, @@ -27,6 +25,7 @@ import { writeDeleteSet, createDeleteSetFromStructStore, transact, + readItem, Doc, Transaction, GC, Item, StructStore, ID // eslint-disable-line } from '../internals.js' @@ -80,7 +79,9 @@ export const writeClientsStructs = (encoder, store, _sm) => { }) // write # states that were updated encoding.writeVarUint(encoder, sm.size) - sm.forEach((clock, client) => { + // Write items with higher client ids first + // This heavily improves the conflict algorithm. + Array.from(sm.entries()).sort((a, b) => b[0] - a[0]).forEach(([client, clock]) => { // @ts-ignore writeStructs(encoder, store.clients.get(client), client, clock) }) @@ -88,13 +89,14 @@ export const writeClientsStructs = (encoder, store, _sm) => { /** * @param {decoding.Decoder} decoder The decoder object to read data from. - * @param {Map>} clientRefs - * @return {Map>} + * @param {Map>} clientRefs + * @param {Doc} doc + * @return {Map>} * * @private * @function */ -export const readClientsStructRefs = (decoder, clientRefs) => { +export const readClientsStructRefs = (decoder, clientRefs, doc) => { const numOfStateUpdates = decoding.readVarUint(decoder) for (let i = 0; i < numOfStateUpdates; i++) { const numberOfStructs = decoding.readVarUint(decoder) @@ -102,15 +104,16 @@ export const readClientsStructRefs = (decoder, clientRefs) => { const nextIdClient = nextID.client let nextIdClock = nextID.clock /** - * @type {Array} + * @type {Array} */ const refs = [] clientRefs.set(nextIdClient, refs) for (let i = 0; i < numberOfStructs; i++) { const info = decoding.readUint8(decoder) - const ref = (binary.BITS5 & info) === 0 ? new GCRef(decoder, createID(nextIdClient, nextIdClock), info) : new ItemRef(decoder, createID(nextIdClient, nextIdClock), info) - refs.push(ref) - nextIdClock += ref.length + const id = createID(nextIdClient, nextIdClock) + const struct = (binary.BITS5 & info) === 0 ? new GC(id, decoding.readVarUint(decoder)) : readItem(decoder, id, info, doc) + refs.push(struct) + nextIdClock += struct.length } } return clientRefs @@ -155,12 +158,12 @@ const resumeStructIntegration = (transaction, store) => { } } const ref = stack[stack.length - 1] - const m = ref._missing const refID = ref.id const client = refID.client const refClock = refID.clock const localClock = getState(store, client) const offset = refClock < localClock ? localClock - refClock : 0 + const missing = ref.getMissing(transaction, store) if (refClock + offset !== localClock) { // A previous message from this client is missing // check if there is a pending structRef with a smaller clock and switch them @@ -180,27 +183,21 @@ const resumeStructIntegration = (transaction, store) => { // wait until missing struct is available return } - while (m.length > 0) { - const missing = m[m.length - 1] - if (getState(store, missing.client) <= missing.clock) { - const client = missing.client - // get the struct reader that has the missing struct - const structRefs = clientsStructRefs.get(client) - if (structRefs === undefined) { - // This update message causally depends on another update message. - return - } - stack.push(structRefs.refs[structRefs.i++]) - if (structRefs.i === structRefs.refs.length) { - clientsStructRefs.delete(client) - } - break + if (missing) { + const client = missing.client + // get the struct reader that has the missing struct + const structRefs = clientsStructRefs.get(client) + if (structRefs === undefined) { + // This update message causally depends on another update message. + return } - ref._missing.pop() - } - if (m.length === 0) { + stack.push(structRefs.refs[structRefs.i++]) + if (structRefs.i === structRefs.refs.length) { + clientsStructRefs.delete(client) + } + } else { if (offset < ref.length) { - ref.toStruct(transaction, store, offset).integrate(transaction) + ref.integrate(transaction, offset) } stack.pop() } @@ -233,7 +230,7 @@ export const writeStructsFromTransaction = (encoder, transaction) => writeClient /** * @param {StructStore} store - * @param {Map>} clientsStructsRefs + * @param {Map>} clientsStructsRefs * * @private * @function @@ -270,7 +267,7 @@ const mergeReadStructsIntoPendingReads = (store, clientsStructsRefs) => { */ export const readStructs = (decoder, transaction, store) => { const clientsStructRefs = new Map() - readClientsStructRefs(decoder, clientsStructRefs) + readClientsStructRefs(decoder, clientsStructRefs, transaction.doc) mergeReadStructsIntoPendingReads(store, clientsStructRefs) resumeStructIntegration(transaction, store) tryResumePendingDeleteReaders(transaction, store) diff --git a/src/utils/isParentOf.js b/src/utils/isParentOf.js index ba3ebb5e..d3012e24 100644 --- a/src/utils/isParentOf.js +++ b/src/utils/isParentOf.js @@ -16,7 +16,7 @@ export const isParentOf = (parent, child) => { if (child.parent === parent) { return true } - child = child.parent._item + child = /** @type {AbstractType} */ (child.parent)._item } return false } diff --git a/tests/y-map.tests.js b/tests/y-map.tests.js index cb38b0ee..00d01172 100644 --- a/tests/y-map.tests.js +++ b/tests/y-map.tests.js @@ -454,7 +454,7 @@ const mapTransactions = [ * @param {t.TestCase} tc */ export const testRepeatGeneratingYmapTests10 = tc => { - applyRandomTests(tc, mapTransactions, 10) + applyRandomTests(tc, mapTransactions, 3) } /** diff --git a/tests/y-text.tests.js b/tests/y-text.tests.js index 13f8d62f..b49a6fab 100644 --- a/tests/y-text.tests.js +++ b/tests/y-text.tests.js @@ -209,16 +209,28 @@ export const testFormattingRemovedInMidText = tc => { * @param {t.TestCase} tc */ export const testLargeFragmentedDocument = tc => { - const { text0, testConnector } = init(tc, { users: 2 }) - // @ts-ignore - text0.doc.transact(() => { - for (let i = 0; i < 1000000; i++) { - text0.insert(0, '0') - } - }) - t.measureTime('time to apply', () => { - testConnector.flushAllMessages() - }) + const itemsToInsert = 1000000 + let update = /** @type {any} */ (null) + ;(() => { + const doc1 = new Y.Doc() + const text0 = doc1.getText('txt') + t.measureTime(`time to insert ${itemsToInsert}`, () => { + doc1.transact(() => { + for (let i = 0; i < itemsToInsert; i++) { + text0.insert(0, '0') + } + }) + }) + t.measureTime('time to encode', () => { + update = Y.encodeStateAsUpdate(doc1) + }) + })() + ;(() => { + const doc2 = new Y.Doc() + t.measureTime('time to apply', () => { + Y.applyUpdate(doc2, update) + }) + })() } // RANDOM TESTS