From 88883a74025586d61f644c1d2f53a3454c25b717 Mon Sep 17 00:00:00 2001
From: Bartosz Sypytkowski <b.sypytkowski@gmail.com>
Date: Fri, 30 Jun 2023 11:29:06 +0200
Subject: [PATCH] moved linkedBy reference out of Item into StructStore
---
src/structs/ContentType.js | 7 ++--
src/structs/Item.js | 64 ++++++++++++++++++++++++++-----------
src/types/AbstractType.js | 17 ++++++----
src/types/YWeakLink.js | 61 ++++++++++++++++++++++++++++-------
src/utils/StructStore.js | 10 +++++-
tests/index.js | 3 +-
tests/y-weak-links.tests.js | 2 --
7 files changed, 120 insertions(+), 44 deletions(-)
diff --git a/src/structs/ContentType.js b/src/structs/ContentType.js
index 42ada06e..cc079694 100644
--- a/src/structs/ContentType.js
+++ b/src/structs/ContentType.js
@@ -9,7 +9,8 @@ import {
readYXmlHook,
readYXmlText,
UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction, Item, YEvent, AbstractType, // eslint-disable-line
- readYWeakLink
+ readYWeakLink,
+ unlinkFrom
} from '../internals.js'
import * as error from 'lib0/error'
@@ -114,8 +115,8 @@ export class ContentType {
const type = /** @type {WeakLink<any>} */ (this.type);
if (type._linkedItem !== null && !type._linkedItem.deleted) {
const item = /** @type {Item} */ (type._linkedItem)
- if (item.linkedBy !== null) {
- item.linkedBy.delete(type)
+ if (item.linked) {
+ unlinkFrom(transaction, item, type)
}
type._linkedItem = null
}
diff --git a/src/structs/Item.js b/src/structs/Item.js
index 6ea9903e..e8a91f32 100644
--- a/src/structs/Item.js
+++ b/src/structs/Item.js
@@ -296,13 +296,6 @@ export class Item extends AbstractStruct {
* @type {ID | null}
*/
this.redone = null
- /**
- * If this item was referenced by other weak links, here we keep the references
- * to these weak refs.
- *
- * @type {Set<YWeakLink<any>> | null}
- */
- this.linkedBy = null
/**
* @type {AbstractContent}
*/
@@ -312,11 +305,28 @@ export class Item extends AbstractStruct {
* bit2: countable
* bit3: deleted
* bit4: mark - mark node as fast-search-marker
+ * bit5: linked - this item is linked by Weak Link references
* @type {number} byte
*/
this.info = this.content.isCountable() ? binary.BIT2 : 0
}
+ /**
+ * This is used to mark the item as linked by weak link references.
+ * Reference dependencies are being kept in StructStore.
+ *
+ * @type {boolean}
+ */
+ set linked(isLinked) {
+ if (((this.info & binary.BIT9) > 0) !== isLinked) {
+ this.info ^= binary.BIT9
+ }
+ }
+
+ get linked() {
+ return (this.info & binary.BIT9) > 0
+ }
+
/**
* This is used to mark the item as an indexed fast-search marker
*
@@ -524,8 +534,16 @@ export class Item extends AbstractStruct {
// set as current parent value if right === null and this is parentSub
/** @type {AbstractType<any>} */ (this.parent)._map.set(this.parentSub, this)
if (this.left !== null) {
- // inherit links from block we're overriding
- this.linkedBy = this.left.linkedBy
+ // move links from block we're overriding
+ this.linked = this.left.linked
+ this.left.linked = false
+ const allLinks = transaction.doc.store.linkedBy
+ const links = allLinks.get(this.left)
+ if (links !== undefined) {
+ allLinks.set(this, links)
+ // since left is being deleted, it will remove
+ // its links from store.linkedBy anyway
+ }
// this is the current attribute value of parent. delete right
this.left.delete(transaction)
}
@@ -538,9 +556,13 @@ export class Item extends AbstractStruct {
this.content.integrate(transaction, this)
// add parent to transaction.changed
addChangedTypeToTransaction(transaction, /** @type {AbstractType<any>} */ (this.parent), this.parentSub)
- if (this.linkedBy !== null) {
- for (let link of this.linkedBy) {
- addChangedTypeToTransaction(transaction, link, this.parentSub)
+ if (this.linked) {
+ // notify links about changes
+ const linkedBy = transaction.doc.store.linkedBy.get(this)
+ if (linkedBy !== undefined) {
+ for (let link of linkedBy) {
+ addChangedTypeToTransaction(transaction, link, this.parentSub)
+ }
}
}
if ((/** @type {AbstractType<any>} */ (this.parent)._item !== null && /** @type {AbstractType<any>} */ (this.parent)._item.deleted) || (this.parentSub !== null && this.right !== null)) {
@@ -600,8 +622,7 @@ export class Item extends AbstractStruct {
this.deleted === right.deleted &&
this.redone === null &&
right.redone === null &&
- this.linkedBy === null &&
- right.linkedBy === null &&
+ !this.linked && !right.linked && // linked items cannot be merged
this.content.constructor === right.content.constructor &&
this.content.mergeWith(right.content)
) {
@@ -647,11 +668,18 @@ export class Item extends AbstractStruct {
addToDeleteSet(transaction.deleteSet, this.id.client, this.id.clock, this.length)
addChangedTypeToTransaction(transaction, parent, this.parentSub)
this.content.delete(transaction)
- if (this.linkedBy !== null) {
- for (let link of this.linkedBy) {
- addChangedTypeToTransaction(transaction, link, this.parentSub)
+
+ if (this.linked) {
+ // notify links that current element has been removed
+ const allLinks = transaction.doc.store.linkedBy
+ const linkedBy = allLinks.get(this)
+ if (linkedBy !== undefined) {
+ for (let link of linkedBy) {
+ addChangedTypeToTransaction(transaction, link, this.parentSub)
+ }
+ allLinks.delete(this)
}
- this.linkedBy = null
+ this.linked = false
}
}
}
diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js
index b8b0e610..d2fa5b67 100644
--- a/src/types/AbstractType.js
+++ b/src/types/AbstractType.js
@@ -243,13 +243,16 @@ export const callTypeObservers = (type, transaction, event, visitedLinks = null)
map.setIfUndefined(changedParentTypes, type, () => []).push(event)
if (type._item === null) {
break
- } else if (type._item.linkedBy !== null) {
- for (let link of type._item.linkedBy) {
- if (visitedLinks === null || !visitedLinks.has(link)) {
- visitedLinks = visitedLinks !== null ? visitedLinks : new Set()
- visitedLinks.add(link)
- // recursive call
- callTypeObservers(link, transaction, /** @type {any} */ (event), visitedLinks)
+ } else if (type._item.linked) {
+ const linkedBy = transaction.doc.store.linkedBy.get(type._item)
+ if (linkedBy !== undefined) {
+ for (let link of linkedBy) {
+ if (visitedLinks === null || !visitedLinks.has(link)) {
+ visitedLinks = visitedLinks !== null ? visitedLinks : new Set()
+ visitedLinks.add(link)
+ // recursive call
+ callTypeObservers(link, transaction, /** @type {any} */ (event), visitedLinks)
+ }
}
}
}
diff --git a/src/types/YWeakLink.js b/src/types/YWeakLink.js
index 2953aaff..f362d89a 100644
--- a/src/types/YWeakLink.js
+++ b/src/types/YWeakLink.js
@@ -1,4 +1,6 @@
import { decoding, encoding, error } from "lib0"
+import * as map from 'lib0/map'
+import * as set from 'lib0/set'
import {
YEvent, Transaction, ID, GC, AbstractType, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Item,
transact,
@@ -92,11 +94,7 @@ export class YWeakLink extends AbstractType {
}
this._linkedItem = sourceItem
if (!sourceItem.deleted) {
- const src = /** @type {Item} */ (sourceItem)
- if (src.linkedBy === null) {
- src.linkedBy = new Set()
- }
- src.linkedBy.add(this)
+ createLink(transaction, /** @type {Item} */ (sourceItem), this)
}
})
}
@@ -171,10 +169,12 @@ export const arrayWeakLink = (transaction, parent, index) => {
item = getItemCleanEnd(transaction, transaction.doc.store, createID(item.id.client, item.id.clock))
}
const link = new YWeakLink(item.id, item)
- if (item.linkedBy === null) {
- item.linkedBy = new Set()
+ if (parent.doc !== null) {
+ const source = /** @type {Item} */ (item)
+ transact(parent.doc, (transaction) => {
+ createLink(transaction, source, link)
+ })
}
- item.linkedBy.add(link)
return link
}
index -= item.length
@@ -195,12 +195,51 @@ export const mapWeakLink = (parent, key) => {
const item = parent._map.get(key)
if (item !== undefined) {
const link = new YWeakLink(item.id, item)
- if (item.linkedBy === null) {
- item.linkedBy = new Set()
+ if (parent.doc !== null) {
+ transact(parent.doc, (transaction) => {
+ createLink(transaction, item, link)
+ })
}
- item.linkedBy.add(link)
return link
} else {
return undefined
}
+}
+
+/**
+ * Establishes a link between source and weak link reference.
+ * It assumes that source has already been split if necessary.
+ *
+ * @param {Transaction} transaction
+ * @param {Item} source
+ * @param {YWeakLink<any>} linkRef
+ */
+export const createLink = (transaction, source, linkRef) => {
+ const allLinks = transaction.doc.store.linkedBy
+ map.setIfUndefined(allLinks, source, set.create).add(linkRef)
+ source.linked = true
+}
+
+/**
+ * Deletes the link between source and a weak link reference.
+ *
+ * @param {Transaction} transaction
+ * @param {Item} source
+ * @param {YWeakLink<any>} linkRef
+ */
+export const unlinkFrom = (transaction, source, linkRef) => {
+ const allLinks = transaction.doc.store.linkedBy
+ const linkedBy = allLinks.get(source)
+ if (linkedBy !== undefined) {
+ linkedBy.delete(linkRef)
+ if (linkedBy.size === 0) {
+ allLinks.delete(source)
+ source.linked = false
+ if (source.countable) {
+ // since linked property is blocking items from merging,
+ // it may turn out that source item can be merged now
+ transaction._mergeStructs.push(source)
+ }
+ }
+ }
}
\ No newline at end of file
diff --git a/src/utils/StructStore.js b/src/utils/StructStore.js
index 7a2e256c..4b6edc1a 100644
--- a/src/utils/StructStore.js
+++ b/src/utils/StructStore.js
@@ -2,7 +2,8 @@
import {
GC,
splitItem,
- Transaction, ID, Item, DSDecoderV2 // eslint-disable-line
+ Transaction, ID, Item, DSDecoderV2, // eslint-disable-line
+ YWeakLink
} from '../internals.js'
import * as math from 'lib0/math'
@@ -14,6 +15,13 @@ export class StructStore {
* @type {Map<number,Array<GC|Item>>}
*/
this.clients = new Map()
+ /**
+ * If this item was referenced by other weak links, here we keep the references
+ * to these weak refs.
+ *
+ * @type {Map<Item, Set<YWeakLink<any>>>}
+ */
+ this.linkedBy = new Map()
/**
* @type {null | { missing: Map<number, number>, update: Uint8Array }}
*/
diff --git a/tests/index.js b/tests/index.js
index ab596392..653908a7 100644
--- a/tests/index.js
+++ b/tests/index.js
@@ -21,8 +21,7 @@ if (isBrowser) {
log.createVConsole(document.body)
}
runTests({
- //doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions,
- weakLinks
+ doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, weakLinks
}).then(success => {
/* istanbul ignore next */
if (isNode) {
diff --git a/tests/y-weak-links.tests.js b/tests/y-weak-links.tests.js
index 21b2dcba..56c2146d 100644
--- a/tests/y-weak-links.tests.js
+++ b/tests/y-weak-links.tests.js
@@ -488,9 +488,7 @@ export const testRemoteMapUpdate = tc => {
map0.set('key', 3)
// apply updated content first, link second
- console.log('update U0 -> U2')
Y.applyUpdate(users[2], Y.encodeStateAsUpdate(users[0]))
- console.log('update U1 -> U2')
Y.applyUpdate(users[2], Y.encodeStateAsUpdate(users[1]))
// make sure that link can find the most recent block