yonote/yjs
23
0
Fork 0
Code Pull Requests 1 Releases Activity

Merge deleted items more efficiently.

Browse Source 
Previously deleted items were simply added to transaction._mergeStructs. But this inherently inefficient as it will splice the struct store for every item.

Now Yjs iterates over transaction.ds and tries to merge structs. It iterates from right to left so merging should be more efficient that before. But more work needs to be done.

For example we could set structs[i] = null and filter the structs after merging is done.
This commit is contained in:
Kevin Jahns 2019-04-28 17:20:35 +02:00
parent c9dda245bf
commit 20005eecdb
3 changed files with 67 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/structs/AbstractItem.js
View File

@ -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
}
}

13
src/structs/ItemType.js
View File

@ -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)

69
src/utils/Transaction.js
View File

@ -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