diff --git a/src/document/change/context.ts b/src/document/change/context.ts index 54775b895..d097006d4 100644 --- a/src/document/change/context.ts +++ b/src/document/change/context.ts @@ -22,7 +22,6 @@ import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { CRDTContainer, CRDTElement, - CRDTGCElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { Operation } from '@yorkie-js-sdk/src/document/operation/operation'; import { ChangeID } from '@yorkie-js-sdk/src/document/change/change_id'; @@ -103,14 +102,6 @@ export class ChangeContext

{ this.root.registerRemovedElement(deleted); } - /** - * `registerElementHasRemovedNodes` register GC element has removed node for - * garbage collection. - */ - public registerElementHasRemovedNodes(elem: CRDTGCElement): void { - this.root.registerElementHasRemovedNodes(elem); - } - /** * `registerGCPair` registers the given pair to hash table. */ diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 463bfb732..d9dd6a6b4 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -157,11 +157,3 @@ export abstract class CRDTContainer extends CRDTElement { */ abstract getByID(createdAt: TimeTicket): CRDTElement | undefined; } - -/** - * `CRDTGCElement` represents element which has garbage collecting method. - */ -export abstract class CRDTGCElement extends CRDTElement { - abstract getRemovedNodesLen(): number; - abstract purgeRemovedNodesBefore(ticket: TimeTicket): number; -} diff --git a/src/document/crdt/rga_tree_split.ts b/src/document/crdt/rga_tree_split.ts index 7ce6e1a8f..1a0014169 100644 --- a/src/document/crdt/rga_tree_split.ts +++ b/src/document/crdt/rga_tree_split.ts @@ -25,6 +25,7 @@ import { TimeTicket, TimeTicketStruct, } from '@yorkie-js-sdk/src/document/time/ticket'; +import { GCChild, GCPair, GCParent } from '@yorkie-js-sdk/src/document/crdt/gc'; export interface ValueChange { actor: ActorID; @@ -243,9 +244,10 @@ export type RGATreeSplitPosRange = [RGATreeSplitPos, RGATreeSplitPos]; /** * `RGATreeSplitNode` is a node of RGATreeSplit. */ -export class RGATreeSplitNode< - T extends RGATreeSplitValue, -> extends SplayNode { +export class RGATreeSplitNode + extends SplayNode + implements GCChild +{ private id: RGATreeSplitNodeID; private removedAt?: TimeTicket; @@ -438,10 +440,15 @@ export class RGATreeSplitNode< * `canDelete` checks if node is able to delete. */ public canDelete(editedAt: TimeTicket, maxCreatedAt: TimeTicket): boolean { - return ( + const justRemoved = !this.removedAt; + if ( !this.getCreatedAt().after(maxCreatedAt) && (!this.removedAt || editedAt.after(this.removedAt)) - ); + ) { + return justRemoved; + } + + return false; } /** @@ -491,6 +498,13 @@ export class RGATreeSplitNode< this.value = value.substring(0, offset) as T; return value.substring(offset, value.length) as T; } + + /** + * `toIDString` returns a string that can be used as an ID for this position. + */ + public toIDString(): string { + return this.id.toIDString(); + } } /** @@ -500,17 +514,15 @@ export class RGATreeSplitNode< * the block is split. * */ -export class RGATreeSplit { +export class RGATreeSplit implements GCParent { private head: RGATreeSplitNode; private treeByIndex: SplayTree; private treeByID: LLRBTree>; - private removedNodeMap: Map>; constructor() { this.head = RGATreeSplitNode.create(InitialRGATreeSplitNodeID); this.treeByIndex = new SplayTree(); this.treeByID = new LLRBTree(RGATreeSplitNode.createComparator()); - this.removedNodeMap = new Map(); this.treeByIndex.insert(this.head); this.treeByID.put(this.head.getID(), this.head); @@ -533,22 +545,30 @@ export class RGATreeSplit { * @param editedAt - edited time * @param value - value * @param maxCreatedAtMapByActor - maxCreatedAtMapByActor - * @returns `[RGATreeSplitPos, Map, Array]` + * @returns `[RGATreeSplitPos, Map, Array, Array]` */ public edit( range: RGATreeSplitPosRange, editedAt: TimeTicket, value?: T, maxCreatedAtMapByActor?: Map, - ): [RGATreeSplitPos, Map, Array>] { + ): [ + RGATreeSplitPos, + Map, + Array, + Array>, + ] { // 01. split nodes with from and to const [toLeft, toRight] = this.findNodeWithSplit(range[1], editedAt); const [fromLeft, fromRight] = this.findNodeWithSplit(range[0], editedAt); // 02. delete between from and to const nodesToDelete = this.findBetween(fromRight, toRight); - const [changes, maxCreatedAtMap, removedNodeMapByNodeKey] = - this.deleteNodes(nodesToDelete, editedAt, maxCreatedAtMapByActor); + const [changes, maxCreatedAtMap, removedNodes] = this.deleteNodes( + nodesToDelete, + editedAt, + maxCreatedAtMapByActor, + ); const caretID = toRight ? toRight.getID() : toLeft.getID(); let caretPos = RGATreeSplitPos.of(caretID, 0); @@ -580,11 +600,12 @@ export class RGATreeSplit { } // 04. add removed node - for (const [key, removedNode] of removedNodeMapByNodeKey) { - this.removedNodeMap.set(key, removedNode); + const pairs: Array = []; + for (const [, removedNode] of removedNodes) { + pairs.push({ parent: this, child: removedNode }); } - return [caretPos, maxCreatedAtMap, changes]; + return [caretPos, maxCreatedAtMap, pairs, changes]; } /** @@ -865,7 +886,7 @@ export class RGATreeSplit { ); const createdAtMapByActor = new Map(); - const removedNodeMap = new Map(); + const removedNodes = new Map(); // First we need to collect indexes for change. const changes = this.makeChanges(nodesToKeep, editedAt); for (const node of nodesToDelete) { @@ -877,13 +898,13 @@ export class RGATreeSplit { ) { createdAtMapByActor.set(actorID, node.getID().getCreatedAt()); } - removedNodeMap.set(node.getID().toIDString(), node); + removedNodes.set(node.getID().toIDString(), node); node.remove(editedAt); } // Finally remove index nodes of tombstones. this.deleteIndexNodes(nodesToKeep); - return [changes, createdAtMapByActor, removedNodeMap]; + return [changes, createdAtMapByActor, removedNodes]; } private filterNodes( @@ -987,31 +1008,6 @@ export class RGATreeSplit { } } - /** - * `getRemovedNodesLen` returns size of removed nodes. - */ - public getRemovedNodesLen(): number { - return this.removedNodeMap.size; - } - - /** - * `purgeRemovedNodesBefore` physically purges nodes that have been removed. - */ - public purgeRemovedNodesBefore(ticket: TimeTicket): number { - let count = 0; - for (const [, node] of this.removedNodeMap) { - if (node.getRemovedAt() && ticket.compare(node.getRemovedAt()!) >= 0) { - this.treeByIndex.delete(node); - this.purge(node); - this.treeByID.remove(node.getID()); - this.removedNodeMap.delete(node.getID().toIDString()); - count++; - } - } - - return count; - } - /** * `purge` physically purges the given node from RGATreeSplit. */ diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts index 91a2d711b..8b5d5b122 100644 --- a/src/document/crdt/rht.ts +++ b/src/document/crdt/rht.ts @@ -269,6 +269,7 @@ export class RHT { public purge(child: RHTNode) { const node = this.nodeMapByKey.get(child.getKey()); if (node == undefined || node.toIDString() != child.toIDString()) { + // TODO(hackerwins): Should we return an error when the child is not found? return; } diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts index 5c2f19031..efe2305af 100644 --- a/src/document/crdt/root.ts +++ b/src/document/crdt/root.ts @@ -22,10 +22,11 @@ import { import { CRDTContainer, CRDTElement, - CRDTGCElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object'; import { GCPair } from '@yorkie-js-sdk/src/document/crdt/gc'; +import { CRDTText } from '@yorkie-js-sdk/src/document/crdt/text'; +import { CRDTTree } from '@yorkie-js-sdk/src/document/crdt/tree'; /** * `CRDTElementPair` is a structure that represents a pair of element and its @@ -58,18 +59,11 @@ export class CRDTRoot { private elementPairMapByCreatedAt: Map; /** - * `removedElementSetByCreatedAt` is a hash set that contains the creation + * `gcElementSetByCreatedAt` is a hash set that contains the creation * time of the removed element. It is used to find the removed element when * executing garbage collection. */ - private removedElementSetByCreatedAt: Set; - - /** - * `elementHasRemovedNodesSetByCreatedAt` is a hash set that contains the - * creation time of the element that has removed nodes. It is used to find - * the element that has removed nodes when executing garbage collection. - */ - private elementHasRemovedNodesSetByCreatedAt: Set; + private gcElementSetByCreatedAt: Set; /** * `gcPairMap` is a hash table that maps the IDString of GCChild to the @@ -80,10 +74,21 @@ export class CRDTRoot { constructor(rootObject: CRDTObject) { this.rootObject = rootObject; this.elementPairMapByCreatedAt = new Map(); - this.removedElementSetByCreatedAt = new Set(); - this.elementHasRemovedNodesSetByCreatedAt = new Set(); + this.gcElementSetByCreatedAt = new Set(); this.gcPairMap = new Map(); this.registerElement(rootObject, undefined); + + rootObject.getDescendants((elem) => { + if (elem.getRemovedAt()) { + this.registerRemovedElement(elem); + } + if (elem instanceof CRDTText || elem instanceof CRDTTree) { + for (const pair of elem.getGCPairs()) { + this.registerGCPair(pair); + } + } + return false; + }); } /** @@ -175,7 +180,7 @@ export class CRDTRoot { const deregisterElementInternal = (elem: CRDTElement) => { const createdAt = elem.getCreatedAt().toIDString(); this.elementPairMapByCreatedAt.delete(createdAt); - this.removedElementSetByCreatedAt.delete(createdAt); + this.gcElementSetByCreatedAt.delete(createdAt); count++; }; @@ -194,17 +199,7 @@ export class CRDTRoot { * `registerRemovedElement` registers the given element to the hash set. */ public registerRemovedElement(element: CRDTElement): void { - this.removedElementSetByCreatedAt.add(element.getCreatedAt().toIDString()); - } - - /** - * `registerElementHasRemovedNodes` registers the given GC element to the - * hash set. - */ - public registerElementHasRemovedNodes(elem: CRDTGCElement): void { - this.elementHasRemovedNodesSetByCreatedAt.add( - elem.getCreatedAt().toIDString(), - ); + this.gcElementSetByCreatedAt.add(element.getCreatedAt().toIDString()); } /** @@ -228,27 +223,12 @@ export class CRDTRoot { } /** - * `getRemovedElementSetSize()` returns the size of removed element set. + * `getGarbageElementSetSize()` returns the size of removed element set. */ - public getRemovedElementSetSize(): number { - return this.removedElementSetByCreatedAt.size; - } - - /** - * `getObject` returns root object. - */ - public getObject(): CRDTObject { - return this.rootObject; - } - - /** - * `getGarbageLen` returns length of nodes which can be garbage collected. - */ - public getGarbageLen(): number { - let count = 0; + public getGarbageElementSetSize(): number { const seen = new Set(); - for (const createdAt of this.removedElementSetByCreatedAt) { + for (const createdAt of this.gcElementSetByCreatedAt) { seen.add(createdAt); const pair = this.elementPairMapByCreatedAt.get(createdAt)!; if (pair.element instanceof CRDTContainer) { @@ -258,18 +238,21 @@ export class CRDTRoot { }); } } + return seen.size; + } - count += seen.size; - - for (const createdAt of this.elementHasRemovedNodesSetByCreatedAt) { - const pair = this.elementPairMapByCreatedAt.get(createdAt)!; - const elem = pair.element as CRDTGCElement; - count += elem.getRemovedNodesLen(); - } - - count += this.gcPairMap.size; + /** + * `getObject` returns root object. + */ + public getObject(): CRDTObject { + return this.rootObject; + } - return count; + /** + * `getGarbageLen` returns length of nodes which can be garbage collected. + */ + public getGarbageLen(): number { + return this.getGarbageElementSetSize() + this.gcPairMap.size; } /** @@ -285,7 +268,7 @@ export class CRDTRoot { public garbageCollect(ticket: TimeTicket): number { let count = 0; - for (const createdAt of this.removedElementSetByCreatedAt) { + for (const createdAt of this.gcElementSetByCreatedAt) { const pair = this.elementPairMapByCreatedAt.get(createdAt)!; if ( pair.element.getRemovedAt() && @@ -296,27 +279,14 @@ export class CRDTRoot { } } - for (const createdAt of this.elementHasRemovedNodesSetByCreatedAt) { - const pair = this.elementPairMapByCreatedAt.get(createdAt)!; - const elem = pair.element as CRDTGCElement; - - const removedNodeCnt = elem.purgeRemovedNodesBefore(ticket); - if (elem.getRemovedNodesLen() === 0) { - this.elementHasRemovedNodesSetByCreatedAt.delete( - elem.getCreatedAt().toIDString(), - ); - } - count += removedNodeCnt; - } - for (const [, pair] of this.gcPairMap) { const removedAt = pair.child.getRemovedAt(); if (removedAt !== undefined && ticket.compare(removedAt) >= 0) { pair.parent.purge(pair.child); - } - this.gcPairMap.delete(pair.child.toIDString()); - count += 1; + this.gcPairMap.delete(pair.child.toIDString()); + count += 1; + } } return count; diff --git a/src/document/crdt/text.ts b/src/document/crdt/text.ts index 5e183632b..df5442de1 100644 --- a/src/document/crdt/text.ts +++ b/src/document/crdt/text.ts @@ -21,7 +21,7 @@ import { } from '@yorkie-js-sdk/src/document/time/ticket'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; import { RHT, RHTNode } from '@yorkie-js-sdk/src/document/crdt/rht'; -import { CRDTGCElement } from '@yorkie-js-sdk/src/document/crdt/element'; +import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { RGATreeSplit, RGATreeSplitNode, @@ -168,13 +168,28 @@ export class CRDTTextValue { this.attributes.purge(node); } } + + /** + * `getGCPairs` returns the pairs of GC. + */ + public getGCPairs(): Array { + const pairs = []; + + for (const node of this.attributes) { + if (node.getRemovedAt()) { + pairs.push({ parent: this, child: node }); + } + } + + return pairs; + } } /** * `CRDTText` is a custom CRDT data type to represent the contents of text editors. * */ -export class CRDTText extends CRDTGCElement { +export class CRDTText extends CRDTElement { private rgaTreeSplit: RGATreeSplit; constructor( @@ -206,7 +221,12 @@ export class CRDTText extends CRDTGCElement { editedAt: TimeTicket, attributes?: Record, maxCreatedAtMapByActor?: Map, - ): [Map, Array>, RGATreeSplitPosRange] { + ): [ + Map, + Array>, + Array, + RGATreeSplitPosRange, + ] { const crdtTextValue = content ? CRDTTextValue.create(content) : undefined; if (crdtTextValue && attributes) { for (const [k, v] of Object.entries(attributes)) { @@ -214,12 +234,13 @@ export class CRDTText extends CRDTGCElement { } } - const [caretPos, maxCreatedAtMap, valueChanges] = this.rgaTreeSplit.edit( - range, - editedAt, - crdtTextValue, - maxCreatedAtMapByActor, - ); + const [caretPos, maxCreatedAtMap, pairs, valueChanges] = + this.rgaTreeSplit.edit( + range, + editedAt, + crdtTextValue, + maxCreatedAtMapByActor, + ); const changes: Array> = valueChanges.map((change) => ({ ...change, @@ -235,7 +256,7 @@ export class CRDTText extends CRDTGCElement { type: TextChangeType.Content, })); - return [maxCreatedAtMap, changes, [caretPos, caretPos]]; + return [maxCreatedAtMap, changes, pairs, [caretPos, caretPos]]; } /** @@ -246,6 +267,7 @@ export class CRDTText extends CRDTGCElement { * @param range - range of RGATreeSplitNode * @param attributes - style attributes * @param editedAt - edited time + * @param maxCreatedAtMapByActor - maxCreatedAtMapByActor * @internal */ public setStyle( @@ -422,22 +444,6 @@ export class CRDTText extends CRDTGCElement { return this.rgaTreeSplit.toTestString(); } - /** - * `getRemovedNodesLen` returns length of removed nodes - */ - public getRemovedNodesLen(): number { - return this.rgaTreeSplit.getRemovedNodesLen(); - } - - /** - * `purgeRemovedNodesBefore` purges removed nodes before the given time. - * - * @internal - */ - public purgeRemovedNodesBefore(ticket: TimeTicket): number { - return this.rgaTreeSplit.purgeRemovedNodesBefore(ticket); - } - /** * `deepcopy` copies itself deeply. */ @@ -456,4 +462,22 @@ export class CRDTText extends CRDTGCElement { public findIndexesFromRange(range: RGATreeSplitPosRange): [number, number] { return this.rgaTreeSplit.findIndexesFromRange(range); } + + /** + * `getGCPairs` returns the pairs of GC. + */ + public getGCPairs(): Array { + const pairs = []; + for (const node of this.rgaTreeSplit) { + if (node.getRemovedAt()) { + pairs.push({ parent: this.rgaTreeSplit, child: node }); + } + + for (const p of node.getValue().getGCPairs()) { + pairs.push(p); + } + } + + return pairs; + } } diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 851168d67..6b30b5b70 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -20,7 +20,7 @@ import { TimeTicketStruct, MaxTimeTicket, } from '@yorkie-js-sdk/src/document/time/ticket'; -import { CRDTGCElement } from '@yorkie-js-sdk/src/document/crdt/element'; +import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { IndexTree, @@ -419,7 +419,7 @@ export type TreePosStructRange = [CRDTTreePosStruct, CRDTTreePosStruct]; */ export class CRDTTreeNode extends IndexTreeNode - implements GCParent + implements GCParent, GCChild { id: CRDTTreeNodeID; removedAt?: TimeTicket; @@ -456,6 +456,20 @@ export class CRDTTreeNode } } + /** + * `toIDString` returns the IDString of this node. + */ + toIDString(): string { + return this.id.toIDString(); + } + + /** + * `getRemovedAt` returns the time when this node was removed. + */ + getRemovedAt(): TimeTicket | undefined { + return this.removedAt; + } + /** * `create` creates a new instance of CRDTTreeNode. */ @@ -647,11 +661,29 @@ export class CRDTTreeNode /** * `purge` purges the given child node. */ - public purge(node: GCChild): void { - if (this.attrs && node instanceof RHTNode) { + public purge(node: RHTNode): void { + if (this.attrs) { this.attrs.purge(node); } } + + /** + * `getGCPairs` returns the pairs of GC. + */ + public getGCPairs(): Array { + const pairs: Array = []; + if (!this.attrs) { + return pairs; + } + + for (const node of this.attrs) { + if (node.getRemovedAt()) { + pairs.push({ parent: this, child: node }); + } + } + + return pairs; + } } /** @@ -731,16 +763,14 @@ function toTestTreeNode(node: CRDTTreeNode): TreeNodeForTest { /** * `CRDTTree` is a CRDT implementation of a tree. */ -export class CRDTTree extends CRDTGCElement { +export class CRDTTree extends CRDTElement implements GCParent { private indexTree: IndexTree; private nodeMapByID: LLRBTree; - private removedNodeMap: Map; constructor(root: CRDTTreeNode, createdAt: TimeTicket) { super(createdAt); this.indexTree = new IndexTree(root); this.nodeMapByID = new LLRBTree(CRDTTreeNodeID.createComparator()); - this.removedNodeMap = new Map(); this.indexTree.traverse((node) => { this.nodeMapByID.put(node.id, node); @@ -965,7 +995,7 @@ export class CRDTTree extends CRDTGCElement { editedAt: TimeTicket, issueTimeTicket: (() => TimeTicket) | undefined, maxCreatedAtMapByActor?: Map, - ): [Array, Map] { + ): [Array, Array, Map] { // 01. find nodes from the given range and split nodes. const [fromParent, fromLeft] = this.findNodesAndSplitText( range[0], @@ -1040,10 +1070,11 @@ export class CRDTTree extends CRDTGCElement { ); // 02. Delete: delete the nodes that are marked as removed. + const pairs: Array = []; for (const node of nodesToBeRemoved) { node.remove(editedAt); if (node.isRemoved) { - this.removedNodeMap.set(node.id.toIDString(), node); + pairs.push({ parent: this, child: node }); } } @@ -1095,7 +1126,8 @@ export class CRDTTree extends CRDTGCElement { // make new nodes as tombstone immediately. if (fromParent.isRemoved) { node.remove(editedAt); - this.removedNodeMap.set(node.id.toIDString(), node); + + pairs.push({ parent: this, child: node }); } this.nodeMapByID.put(node.id, node); @@ -1123,7 +1155,7 @@ export class CRDTTree extends CRDTGCElement { } } - return [changes, maxCreatedAtMap]; + return [changes, pairs, maxCreatedAtMap]; } /** @@ -1161,33 +1193,12 @@ export class CRDTTree extends CRDTGCElement { } /** - * `purgeRemovedNodesBefore` physically purges nodes that have been removed. - */ - public purgeRemovedNodesBefore(ticket: TimeTicket) { - const nodesToBeRemoved = new Set(); - - let count = 0; - for (const [, node] of this.removedNodeMap) { - if (node.removedAt && ticket.compare(node.removedAt!) >= 0) { - nodesToBeRemoved.add(node); - count++; - } - } - - for (const node of nodesToBeRemoved) { - node.parent?.removeChild(node); - this.nodeMapByID.remove(node.id); - this.purge(node); - this.removedNodeMap.delete(node.id.toIDString()); - } - - return count; - } - - /** - * `purge` physically purges the given node from RGATreeSplit. + * `purge` physically purges the given node. */ public purge(node: CRDTTreeNode): void { + node.parent?.removeChild(node); + this.nodeMapByID.remove(node.id); + const insPrevID = node.insPrevID; const insNextID = node.insNextID; @@ -1206,18 +1217,29 @@ export class CRDTTree extends CRDTGCElement { } /** - * `findPos` finds the position of the given index in the tree. + * `getGCPairs` returns the pairs of GC. */ - public findPos(index: number, preferText = true): CRDTTreePos { - const treePos = this.indexTree.findTreePos(index, preferText); - return CRDTTreePos.fromTreePos(treePos); + public getGCPairs(): Array { + const pairs: Array = []; + this.indexTree.traverse((node) => { + if (node.getRemovedAt()) { + pairs.push({ parent: this, child: node }); + } + + for (const p of node.getGCPairs()) { + pairs.push(p); + } + }); + + return pairs; } /** - * `getRemovedNodesLen` returns size of removed nodes. + * `findPos` finds the position of the given index in the tree. */ - public getRemovedNodesLen(): number { - return this.removedNodeMap.size; + public findPos(index: number, preferText = true): CRDTTreePos { + const treePos = this.indexTree.findTreePos(index, preferText); + return CRDTTreePos.fromTreePos(treePos); } /** diff --git a/src/document/document.ts b/src/document/document.ts index f222811e0..89cf144d3 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -1274,7 +1274,7 @@ export class Document { logger.debug( `trying to apply ${changes.length} remote changes.` + `elements:${this.root.getElementMapSize()}, ` + - `removeds:${this.root.getRemovedElementSetSize()}`, + `removeds:${this.root.getGarbageElementSetSize()}`, ); } if (logger.isEnabled(LogLevel.Trivial)) { @@ -1296,7 +1296,7 @@ export class Document { logger.debug( `after appling ${changes.length} remote changes.` + `elements:${this.root.getElementMapSize()}, ` + - ` removeds:${this.root.getRemovedElementSetSize()}`, + ` removeds:${this.root.getGarbageElementSetSize()}`, ); } } diff --git a/src/document/json/text.ts b/src/document/json/text.ts index a796c432e..c184f3db9 100644 --- a/src/document/json/text.ts +++ b/src/document/json/text.ts @@ -101,13 +101,17 @@ export class Text { } const attrs = attributes ? stringifyObjectValues(attributes) : undefined; const ticket = this.context.issueTimeTicket(); - const [maxCreatedAtMapByActor, , rangeAfterEdit] = this.text.edit( + const [maxCreatedAtMapByActor, , pairs, rangeAfterEdit] = this.text.edit( range, content, ticket, attrs, ); + for (const pair of pairs) { + this.context!.registerGCPair(pair); + } + this.context.push( new EditOperation( this.text.getCreatedAt(), @@ -120,10 +124,6 @@ export class Text { ), ); - if (!range[0].equals(range[1])) { - this.context.registerElementHasRemovedNodes(this.text); - } - return this.text.findIndexesFromRange(rangeAfterEdit); } diff --git a/src/document/json/tree.ts b/src/document/json/tree.ts index e6c335073..c05186378 100644 --- a/src/document/json/tree.ts +++ b/src/document/json/tree.ts @@ -419,7 +419,7 @@ export class Tree { .filter((a) => a) as Array; } - const [, maxCreatedAtMapByActor] = this.tree!.edit( + const [, pairs, maxCreatedAtMapByActor] = this.tree!.edit( [fromPos, toPos], crdtNodes.length ? crdtNodes.map((crdtNode) => crdtNode?.deepcopy()) @@ -429,6 +429,10 @@ export class Tree { () => this.context!.issueTimeTicket(), ); + for (const pair of pairs) { + this.context!.registerGCPair(pair); + } + this.context!.push( TreeEditOperation.create( this.tree!.getCreatedAt(), @@ -441,10 +445,6 @@ export class Tree { ), ); - if (!fromPos.equals(toPos)) { - this.context!.registerElementHasRemovedNodes(this.tree!); - } - return true; } diff --git a/src/document/operation/edit_operation.ts b/src/document/operation/edit_operation.ts index 05ad5c661..6175f9d77 100644 --- a/src/document/operation/edit_operation.ts +++ b/src/document/operation/edit_operation.ts @@ -90,7 +90,7 @@ export class EditOperation extends Operation { } const text = parentObject as CRDTText; - const [, changes] = text.edit( + const [, changes, pairs] = text.edit( [this.fromPos, this.toPos], this.content, this.getExecutedAt(), @@ -98,9 +98,10 @@ export class EditOperation extends Operation { this.maxCreatedAtMapByActor, ); - if (!this.fromPos.equals(this.toPos)) { - root.registerElementHasRemovedNodes(text); + for (const pair of pairs) { + root.registerGCPair(pair); } + return { opInfos: changes.map(({ from, to, value }) => { return { diff --git a/src/document/operation/tree_edit_operation.ts b/src/document/operation/tree_edit_operation.ts index 22443c83b..6a2bb6f82 100644 --- a/src/document/operation/tree_edit_operation.ts +++ b/src/document/operation/tree_edit_operation.ts @@ -92,7 +92,7 @@ export class TreeEditOperation extends Operation { } const editedAt = this.getExecutedAt(); const tree = parentObject as CRDTTree; - const [changes] = tree.edit( + const [changes, pairs] = tree.edit( [this.fromPos, this.toPos], this.contents?.map((content) => content.deepcopy()), this.splitLevel, @@ -120,9 +120,10 @@ export class TreeEditOperation extends Operation { this.maxCreatedAtMapByActor, ); - if (!this.fromPos.equals(this.toPos)) { - root.registerElementHasRemovedNodes(tree); + for (const pair of pairs) { + root.registerGCPair(pair); } + return { opInfos: changes.map( ({ from, to, value, splitLevel, fromPath, toPath }) => { diff --git a/test/unit/document/gc_test.ts b/test/unit/document/gc_test.ts index 8840aff80..fa0aa2ab5 100644 --- a/test/unit/document/gc_test.ts +++ b/test/unit/document/gc_test.ts @@ -155,38 +155,27 @@ describe('Garbage Collection', function () { ); }); - it('should collect garbage for text node 2', function () { + it('should return correct gc count with already removed text node', function () { const doc = new yorkie.Document<{ k1: Text }>('test-doc'); assert.equal(doc.toSortedJSON(), '{}'); - let expectedMessage = '{"k1":[{"val":"Hello "},{"val":"mario"}]}'; doc.update((root) => { root.k1 = new Text(); - root.k1.edit(0, 0, 'Hello world'); - root.k1.edit(6, 11, 'mario'); - assert.equal(root.toJSON!(), expectedMessage); + root.k1.edit(0, 0, 'ab'); + root.k1.edit(0, 1, 'c'); }, 'edit text k1'); - assert.equal(doc.toSortedJSON(), expectedMessage); + assert.equal(doc.toSortedJSON(), '{"k1":[{"val":"c"},{"val":"b"}]}'); assert.equal(doc.getGarbageLen(), 1); - expectedMessage = - '{"k1":[{"val":"Hi"},{"val":" "},{"val":"j"},{"val":"ane"}]}'; - doc.update((root) => { const text = root['k1']; - text.edit(0, 5, 'Hi'); - text.edit(3, 4, 'j'); - text.edit(4, 8, 'ane'); - assert.equal(root.toJSON!(), expectedMessage); + text.edit(1, 2, 'd'); }, 'deletes 2'); - assert.equal(doc.toSortedJSON(), expectedMessage); - - const expectedGarbageLen = 4; - assert.equal(doc.getGarbageLen(), expectedGarbageLen); - assert.equal(doc.garbageCollect(MaxTimeTicket), expectedGarbageLen); + assert.equal(doc.toSortedJSON(), '{"k1":[{"val":"c"},{"val":"d"}]}'); + assert.equal(doc.getGarbageLen(), 2); - const empty = 0; - assert.equal(doc.getGarbageLen(), empty); + assert.equal(doc.garbageCollect(MaxTimeTicket), 2); + assert.equal(doc.getGarbageLen(), 0); }); it('should collect garbage for text node with attributes', function () { @@ -301,6 +290,45 @@ describe('Garbage Collection', function () { assert.equal(nodeLengthBeforeGC - nodeLengthAfterGC, 5); }); + it('should return correct gc count with already removed tree node', () => { + const doc = new yorkie.Document<{ t: Tree }>('test-doc'); + assert.equal(doc.toSortedJSON(), '{}'); + + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [ + { + type: 'tn', + children: [{ type: 'text', value: 'abc' }], + }, + ], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), `

abc

`); + assert.equal(doc.getGarbageLen(), 0); + + doc.update((root) => { + root.t.edit(3, 4, undefined); + }); + assert.equal(doc.getRoot().t.toXML(), `

ac

`); + assert.equal(doc.getGarbageLen(), 1); + + doc.update((root) => { + root.t.edit(2, 4, undefined); + }); + assert.equal(doc.getRoot().t.toXML(), `

`); + assert.equal(doc.getGarbageLen(), 3); + + assert.equal(doc.garbageCollect(MaxTimeTicket), 3); + assert.equal(doc.getGarbageLen(), 0); + }); + it('should collect garbage for nested object', async () => { type TestDoc = { shape?: { point?: { x?: number; y?: number } } }; const doc = new yorkie.Document('test-doc'); @@ -478,7 +506,7 @@ describe('Garbage Collection for tree', () => { } else if (code === OpCode.DeleteNode) { root.t.edit(0, 2, undefined, 0); } else if (code === OpCode.GC) { - doc.garbageCollect(timeT()); + doc.garbageCollect(MaxTimeTicket); } }); assert.equal(doc.getRoot().t.toXML(), expectXML);