diff --git a/src/structs/AbstractItem.js b/src/structs/AbstractItem.js index 614fb4ef..a27b0de7 100644 --- a/src/structs/AbstractItem.js +++ b/src/structs/AbstractItem.js @@ -423,6 +423,8 @@ export class AbstractItem extends AbstractStruct { gcChildren (transaction, store) { } /** + * @todo remove transaction param + * * @param {Transaction} transaction * @param {StructStore} store * @param {boolean} parentGCd @@ -430,7 +432,9 @@ export class AbstractItem extends AbstractStruct { * @private */ gc (transaction, store, parentGCd) { - this.delete(transaction) // @todo: shouldn't be necessary + if (!this.deleted) { + throw error.unexpectedCase() + } let r if (parentGCd) { r = new GC(this.id, this.length) @@ -448,7 +452,6 @@ export class AbstractItem extends AbstractStruct { } } replaceStruct(store, this, r) - transaction._mergeStructs.add(r.id) } /** @@ -616,7 +619,7 @@ export const computeItemParams = (transaction, store, leftid, rightid, parentid, case GC: break default: - if (!parentItem.deleted) { + if (!parentItem.deleted && (left === null || left.constructor !== GC) && (right === null || right.constructor !== GC)) { parent = parentItem.type } } diff --git a/src/structs/ItemType.js b/src/structs/ItemType.js index 57ee6669..bdf648b0 100644 --- a/src/structs/ItemType.js +++ b/src/structs/ItemType.js @@ -106,11 +106,22 @@ export class ItemType extends AbstractItem { while (item !== null) { if (!item.deleted) { item.delete(transaction) + } else { + // Whis will be gc'd later and we want to merge it if possible + // We try to merge all deleted items after each transaction, + // but we have no knowledge about that this needs to be merged + // since it is not in transaction.ds. Hence we add it to transaction._mergeStructs + transaction._mergeStructs.add(item.id) } item = item.right } this.type._map.forEach(item => { - item.delete(transaction) + if (!item.deleted) { + item.delete(transaction) + } else { + // same as above + transaction._mergeStructs.add(item.id) + } }) transaction.changed.delete(this.type) transaction.changedParentTypes.delete(this.type) diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 26d33c7a..41c9cb87 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -176,26 +176,6 @@ export const transact = (y, f, origin = null) => { callEventHandlerListeners(type._dEH, events, transaction) }) y.emit('afterTransaction', [transaction, y]) - // replace deleted items with ItemDeleted / GC - for (const [client, deleteItems] of ds.clients) { - /** - * @type {Array<AbstractStruct>} - */ - // @ts-ignore - const structs = store.clients.get(client) - for (let di = 0; di < deleteItems.length; di++) { - const deleteItem = deleteItems[di] - for (let si = findIndexSS(structs, deleteItem.clock); si < structs.length; si++) { - const struct = structs[si] - if (deleteItem.clock + deleteItem.len <= struct.id.clock) { - break - } - if (struct.deleted && struct instanceof AbstractItem) { - struct.gc(transaction, store, false) - } - } - } - } /** * @param {Array<AbstractStruct>} structs * @param {number} pos @@ -213,6 +193,53 @@ export const transact = (y, f, origin = null) => { } } } + // replace deleted items with ItemDeleted / GC + for (const [client, deleteItems] of ds.clients) { + /** + * @type {Array<AbstractStruct>} + */ + // @ts-ignore + const structs = store.clients.get(client) + for (let di = deleteItems.length - 1; di >= 0; di--) { + const deleteItem = deleteItems[di] + const endDeleteItemClock = deleteItem.clock + deleteItem.len + for ( + let si = findIndexSS(structs, deleteItem.clock), struct = structs[si]; + si < structs.length && struct.id.clock < endDeleteItemClock; + struct = structs[++si] + ) { + const struct = structs[si] + if (deleteItem.clock + deleteItem.len <= struct.id.clock) { + break + } + if (struct.deleted && struct instanceof AbstractItem) { + struct.gc(transaction, store, false) + } + } + } + } + // try to merge deleted / gc'd items + // merge from right to left for better efficiecy and so we don't miss any merge targets + for (const [client, deleteItems] of ds.clients) { + /** + * @type {Array<AbstractStruct>} + */ + // @ts-ignore + const structs = store.clients.get(client) + for (let di = deleteItems.length - 1; di >= 0; di--) { + const deleteItem = deleteItems[di] + // start with merging the item next to the last deleted item + const mostRightIndexToCheck = math.min(structs.length - 1, 1 + findIndexSS(structs, deleteItem.clock + deleteItem.len - 1)) + for ( + let si = mostRightIndexToCheck, struct = structs[si]; + si > 0 && struct.id.clock >= deleteItem.clock; + struct = structs[--si] + ) { + tryToMergeWithLeft(structs, si) + } + } + } + // on all affected store.clients props, try to merge for (const [client, clock] of transaction.afterState) { const beforeClock = transaction.beforeState.get(client) || 0 @@ -230,6 +257,8 @@ export const transact = (y, f, origin = null) => { } } // try to merge mergeStructs + // @todo: it makes more sense to transform mergeStructs to a DS, sort it, and merge from right to left + // but at the moment DS does not handle duplicates for (const mid of transaction._mergeStructs) { const client = mid.client const clock = mid.clock