From bdfea21168e94422e854cf9240ff171919ff5e85 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Tue, 17 Nov 2020 22:02:27 +0900 Subject: [PATCH 01/14] Add garbage collection for text Adds a function to collect garbage of RGATreeSplit that occurs when editing text. --- api/converter/converter_test.go | 3 + pkg/document/change/context.go | 4 + pkg/document/document.go | 2 +- pkg/document/document_test.go | 118 ++++++++++++++++++++++++++++ pkg/document/json/element.go | 7 ++ pkg/document/json/rga_tree_split.go | 77 ++++++++++++++++-- pkg/document/json/root.go | 16 ++++ pkg/document/json/text.go | 11 +++ pkg/document/operation/edit.go | 1 + pkg/document/proxy/text_proxy.go | 1 + 10 files changed, 232 insertions(+), 8 deletions(-) diff --git a/api/converter/converter_test.go b/api/converter/converter_test.go index 8a5735117..bcfb1998c 100644 --- a/api/converter/converter_test.go +++ b/api/converter/converter_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + time2 "github.com/yorkie-team/yorkie/pkg/document/time" + "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/api/converter" @@ -153,6 +155,7 @@ func TestConverter(t *testing.T) { pbPack := converter.ToChangePack(d1.CreateChangePack()) pack, err := converter.FromChangePack(pbPack) + pack.MinSyncedTicket = time2.MaxTicket assert.NoError(t, err) d2 := document.New("c1", "d1") diff --git a/pkg/document/change/context.go b/pkg/document/change/context.go index b03b3aef0..c75fc6329 100644 --- a/pkg/document/change/context.go +++ b/pkg/document/change/context.go @@ -77,3 +77,7 @@ func (c *Context) RegisterElement(elem json.Element) { func (c *Context) RegisterRemovedElementPair(parent json.Container, deleted json.Element) { c.root.RegisterRemovedElementPair(parent, deleted) } + +func (c *Context) RegisterEditedTextElement(textType json.TextElement) { + c.root.RegisterEditedTextElement(textType) +} diff --git a/pkg/document/document.go b/pkg/document/document.go index e1885e4f4..d34a713ad 100644 --- a/pkg/document/document.go +++ b/pkg/document/document.go @@ -37,7 +37,7 @@ import ( // edit the document. type Document struct { // doc is the original data of the actual document. - doc *InternalDocument + doc *InternalDocument // clone is a copy of `doc` to be exposed to the user and is used to // protect `doc`. diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 6a4ab9b34..8991a4161 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -19,6 +19,7 @@ package document_test import ( "errors" "fmt" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -171,6 +172,123 @@ func TestDocument(t *testing.T) { testhelper.PrintMemStats() }) + t.Run("garbage collection for text test", func(t *testing.T) { + doc := document.New("c1", "d1") + assert.Equal(t, "{}", doc.Marshal()) + assert.False(t, doc.HasLocalChanges()) + + // check garbage length + expected := `{"k1":"Hello mario"}` + err := doc.Update(func(root *proxy.ObjectProxy) error { + root.SetNewText("k1"). + Edit(0, 0, "Hello world"). + Edit(6, 11, "mario") + assert.Equal(t, expected, root.Marshal()) + return nil + }, "edit text k1") + assert.NoError(t, err) + assert.Equal(t, expected, doc.Marshal()) + assert.Equal(t, 1, doc.GarbageLen()) + + expected = `{"k1":"Hi jane"}` + err = doc.Update(func(root *proxy.ObjectProxy) error { + text := root.GetText("k1") + text.Edit(0, 5, "Hi") + text.Edit(3, 4, "j") + text.Edit(4, 8, "ane") + assert.Equal(t, expected, root.Marshal()) + return nil + }, "edit text k1") + assert.NoError(t, err) + assert.Equal(t, expected, doc.Marshal()) + + expectedGarbageLen := 4 + assert.Equal(t, expectedGarbageLen, doc.GarbageLen()) + // garbage collect + assert.Equal(t, expectedGarbageLen, doc.GarbageCollect(time.MaxTicket)) + }) + + t.Run("garbage collection for large size of text garbage test", func(t *testing.T) { + doc := document.New("c1", "d1") + assert.Equal(t, "{}", doc.Marshal()) + assert.False(t, doc.HasLocalChanges()) + + textSize := 10 + // 01. initial + err := doc.Update(func(root *proxy.ObjectProxy) error { + text := root.SetNewText("k1") + for i := 0; i < textSize; i++ { + text.Edit(i, i, "a") + } + return nil + }, "initial") + assert.NoError(t, err) + bytes, err := converter.ObjectToBytes(doc.RootObject()) + assert.NoError(t, err) + fmt.Println("-----initial") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + + // 02. 1000 nodes modified + err = doc.Update(func(root *proxy.ObjectProxy) error { + text := root.GetText("k1") + for i := 0; i < textSize; i++ { + text.Edit(i, i+1, "b") + } + return nil + }, "1000 nodes modified") + assert.NoError(t, err) + assert.NoError(t, err) + fmt.Println("-----1000 nodes modified") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + assert.Equal(t, textSize, doc.GarbageLen()) + + assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) + runtime.GC() + fmt.Println("-----Garbage collect") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + assert.NoError(t, err) + + // 03. long text by one node + err = doc.Update(func(root *proxy.ObjectProxy) error { + text := root.SetNewText("k2") + str := "" + for i := 0; i < textSize; i++ { + str += "a" + } + text.Edit(0, 0, str) + return nil + }, "initial") + fmt.Println("-----long text by one node") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + + // 04. Modify one node multiple times + err = doc.Update(func(root *proxy.ObjectProxy) error { + text := root.GetText("k2") + for i := 0; i < textSize; i++ { + if i != textSize { + text.Edit(i, i+1, "b") + } + } + return nil + }, "Modify one node multiple times") + assert.NoError(t, err) + assert.NoError(t, err) + fmt.Println("-----Modify one node multiple times") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + + assert.Equal(t, textSize, doc.GarbageLen()) + assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) + runtime.GC() + fmt.Println("-----Garbage collect") + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + }) + t.Run("object test", func(t *testing.T) { doc := document.New("c1", "d1") err := doc.Update(func(root *proxy.ObjectProxy) error { diff --git a/pkg/document/json/element.go b/pkg/document/json/element.go index e9c40d74c..07f81d21b 100644 --- a/pkg/document/json/element.go +++ b/pkg/document/json/element.go @@ -27,6 +27,13 @@ type Container interface { Descendants(callback func(elem Element, parent Container) bool) } +// TextType represents Text or RichText. +type TextElement interface { + Element + removedNodesLen() int + cleanupRemovedNodes(ticket *time.Ticket) int +} + // Element represents JSON element. type Element interface { // Marshal returns the JSON encoding of this element. diff --git a/pkg/document/json/rga_tree_split.go b/pkg/document/json/rga_tree_split.go index 883a010e1..d8479d806 100644 --- a/pkg/document/json/rga_tree_split.go +++ b/pkg/document/json/rga_tree_split.go @@ -222,10 +222,22 @@ func (t *RGATreeSplitNode) Value() RGATreeSplitValue { return t.value } +// removedNodeMapKey is a key of removedNodeMap. +// The shape of the key is '{createdAt.Key()}:{offset}'. +type removedNodeMapKey string + +// createRemovedNodeMapKey generates keys for removedNodes. +func createRemovedNodeMapKey(createdAtKey string, offset int) removedNodeMapKey { + key := fmt.Sprintf("%s:%d", createdAtKey, offset) + return removedNodeMapKey(key) +} + type RGATreeSplit struct { initialHead *RGATreeSplitNode treeByIndex *splay.Tree treeByID *llrb.Tree + + removedNodeMap map[removedNodeMapKey]*RGATreeSplitNode } func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { @@ -234,9 +246,10 @@ func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { treeByID.Put(initialHead.ID(), initialHead) return &RGATreeSplit{ - initialHead: initialHead, - treeByIndex: treeByIndex, - treeByID: treeByID, + initialHead: initialHead, + treeByIndex: treeByIndex, + treeByID: treeByID, + removedNodeMap: make(map[removedNodeMapKey]*RGATreeSplitNode), } } @@ -373,7 +386,7 @@ func (s *RGATreeSplit) edit( // 02. delete between from and to nodesToDelete := s.findBetween(fromRight, toRight) - latestCreatedAtMap := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) + latestCreatedAtMap, removedNodeMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) var caretID *RGATreeSplitNodeID if toRight == nil { @@ -389,6 +402,11 @@ func (s *RGATreeSplit) edit( caretPos = NewRGATreeSplitNodePos(inserted.id, inserted.contentLen()) } + // 04. add removed node + for key, removedNode := range removedNodeMapByCreatedAt { + s.removedNodeMap[key] = removedNode + } + return caretPos, latestCreatedAtMap } @@ -406,8 +424,9 @@ func (s *RGATreeSplit) deleteNodes( candidates []*RGATreeSplitNode, latestCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, -) map[string]*time.Ticket { +) (map[string]*time.Ticket, map[removedNodeMapKey]*RGATreeSplitNode) { createdAtMapByActor := make(map[string]*time.Ticket) + removedNodeMap := make(map[removedNodeMapKey]*RGATreeSplitNode) for _, node := range candidates { actorIDHex := node.createdAt().ActorIDHex() @@ -426,16 +445,18 @@ func (s *RGATreeSplit) deleteNodes( if node.Remove(editedAt, latestCreatedAt) { s.treeByIndex.Splay(node.indexNode) - latestCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.createdAt if latestCreatedAt == nil || createdAt.After(latestCreatedAt) { createdAtMapByActor[actorIDHex] = createdAt } + + key := createRemovedNodeMapKey(createdAt.Key(), node.id.offset) + removedNodeMap[key] = node } } - return createdAtMapByActor + return createdAtMapByActor, removedNodeMap } func (s *RGATreeSplit) marshal() string { @@ -492,3 +513,45 @@ func (s *RGATreeSplit) AnnotatedString() string { return strings.Join(result, "") } + +// removedNodesLen returns length of removed nodes +func (s *RGATreeSplit) removedNodesLen() int { + return len(s.removedNodeMap) +} + +// cleanupRemovedNodes cleans up nodes that have been removed. +// The cleaned nodes are subject to garbage collector collection. +func (s *RGATreeSplit) cleanupRemovedNodes(ticket *time.Ticket) int { + count := 0 + for _, node := range s.removedNodeMap { + if node.removedAt != nil && ticket.Compare(node.removedAt) >= 0 { + s.treeByIndex.Delete(node.indexNode) + s.purge(node) + s.treeByID.Remove(node.id) + delete(s.removedNodeMap, removedNodeMapKey(node.createdAt().Key())) + count++ + } + } + + return count +} + +// purge removes the node passed as a parameter from RGATreeSplit. +func (s *RGATreeSplit) purge(node *RGATreeSplitNode) { + if node.prev != nil { + node.prev.next = node.next + if node.next != nil { + node.next.prev = node.prev + } + } else { + s.initialHead, node.next.prev = node.next, nil + } + node.next, node.prev = nil, nil + + if node.insPrev != nil { + node.insPrev.insNext, node.insPrev = nil, nil + } + if node.insNext != nil { + node.insNext.insPrev, node.insNext = nil, nil + } +} diff --git a/pkg/document/json/root.go b/pkg/document/json/root.go index aa0198a26..e3c667274 100644 --- a/pkg/document/json/root.go +++ b/pkg/document/json/root.go @@ -35,6 +35,7 @@ type Root struct { object *Object elementMapByCreatedAt map[string]Element removedElementPairMapByCreatedAt map[string]ElementPair + editedTextElementMapByCreatedAt map[string]TextElement } // NewRoot creates a new instance of Root. @@ -42,6 +43,7 @@ func NewRoot(root *Object) *Root { r := &Root{ elementMapByCreatedAt: make(map[string]Element), removedElementPairMapByCreatedAt: make(map[string]ElementPair), + editedTextElementMapByCreatedAt: make(map[string]TextElement), } r.object = root @@ -85,6 +87,11 @@ func (r *Root) RegisterRemovedElementPair(parent Container, elem Element) { } } +// RegisterEditedTextElement register the given text element to hash table. +func (r *Root) RegisterEditedTextElement(textType TextElement) { + r.editedTextElementMapByCreatedAt[textType.CreatedAt().Key()] = textType +} + // DeepCopy copies itself deeply. func (r *Root) DeepCopy() *Root { return NewRoot(r.object.DeepCopy().(*Object)) @@ -101,6 +108,11 @@ func (r *Root) GarbageCollect(ticket *time.Ticket) int { } } + for _, text := range r.editedTextElementMapByCreatedAt { + count += text.cleanupRemovedNodes(ticket) + delete(r.editedTextElementMapByCreatedAt, text.CreatedAt().Key()) + } + return count } @@ -120,6 +132,10 @@ func (r *Root) GarbageLen() int { } } + for _, text := range r.editedTextElementMapByCreatedAt { + count += text.removedNodesLen() + } + return count } diff --git a/pkg/document/json/text.go b/pkg/document/json/text.go index 5781cd31f..ddf75cfd5 100644 --- a/pkg/document/json/text.go +++ b/pkg/document/json/text.go @@ -195,3 +195,14 @@ func (t *Text) Nodes() []*RGATreeSplitNode { func (t *Text) AnnotatedString() string { return t.rgaTreeSplit.AnnotatedString() } + +// removedNodesLen returns length of removed nodes +func (t *Text) removedNodesLen() int { + return t.rgaTreeSplit.removedNodesLen() +} + +// cleanupRemovedNodes cleans up nodes that have been removed. +// The cleaned nodes are subject to garbage collector collection. +func (t *Text) cleanupRemovedNodes(ticket *time.Ticket) int { + return t.rgaTreeSplit.cleanupRemovedNodes(ticket) +} diff --git a/pkg/document/operation/edit.go b/pkg/document/operation/edit.go index db3420790..c45ee362a 100644 --- a/pkg/document/operation/edit.go +++ b/pkg/document/operation/edit.go @@ -54,6 +54,7 @@ func (e *Edit) Execute(root *json.Root) error { switch obj := parent.(type) { case *json.Text: obj.Edit(e.from, e.to, e.latestCreatedAtMapByActor, e.content, e.executedAt) + root.RegisterEditedTextElement(obj) default: return ErrNotApplicableDataType } diff --git a/pkg/document/proxy/text_proxy.go b/pkg/document/proxy/text_proxy.go index 000cc76f8..f19a05ac6 100644 --- a/pkg/document/proxy/text_proxy.go +++ b/pkg/document/proxy/text_proxy.go @@ -62,6 +62,7 @@ func (p *TextProxy) Edit(from, to int, content string) *TextProxy { content, ticket, )) + p.context.RegisterEditedTextElement(p) return p } From ec47b8415f6507db8f49eb0ad164c048960a2dfb Mon Sep 17 00:00:00 2001 From: dc7303 Date: Tue, 17 Nov 2020 22:39:50 +0900 Subject: [PATCH 02/14] Add garbage collection for rich text Adds a function to collect garbage of RGATreeSplit that occurs when editing rich text. --- pkg/document/document_test.go | 42 +++++++++++++++++++++++++-- pkg/document/json/rich_text.go | 11 +++++++ pkg/document/operation/rich_edit.go | 1 + pkg/document/proxy/rich_text_proxy.go | 1 + 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 8991a4161..2ec8f7673 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -208,6 +208,42 @@ func TestDocument(t *testing.T) { assert.Equal(t, expectedGarbageLen, doc.GarbageCollect(time.MaxTicket)) }) + t.Run("garbage collection for rich text test", func(t *testing.T) { + doc := document.New("c1", "d1") + assert.Equal(t, "{}", doc.Marshal()) + assert.False(t, doc.HasLocalChanges()) + + // check garbage length + expected := `{"k1":[{"attrs":{"b":"1"},"val":"Hello "},{"attrs":{},"val":"mario"}]}` + err := doc.Update(func(root *proxy.ObjectProxy) error { + root.SetNewRichText("k1"). + Edit(0, 0, "Hello world", map[string]string{"b": "1"}). + Edit(6, 11, "mario", nil) + assert.Equal(t, expected, root.Marshal()) + return nil + }, "edit text k1") + assert.NoError(t, err) + assert.Equal(t, expected, doc.Marshal()) + assert.Equal(t, 1, doc.GarbageLen()) + + expected = `{"k1":[{"attrs":{"b":"1"},"val":"Hi"},{"attrs":{"b":"1"},"val":" "},{"attrs":{},"val":"j"},{"attrs":{"b":"1"},"val":"ane"}]}` + err = doc.Update(func(root *proxy.ObjectProxy) error { + text := root.GetRichText("k1") + text.Edit(0, 5, "Hi", map[string]string{"b": "1"}) + text.Edit(3, 4, "j", nil) + text.Edit(4, 8, "ane", map[string]string{"b": "1"}) + assert.Equal(t, expected, root.Marshal()) + return nil + }, "edit text k1") + assert.NoError(t, err) + assert.Equal(t, expected, doc.Marshal()) + + expectedGarbageLen := 4 + assert.Equal(t, expectedGarbageLen, doc.GarbageLen()) + // garbage collect + assert.Equal(t, expectedGarbageLen, doc.GarbageCollect(time.MaxTicket)) + }) + t.Run("garbage collection for large size of text garbage test", func(t *testing.T) { doc := document.New("c1", "d1") assert.Equal(t, "{}", doc.Marshal()) @@ -244,6 +280,7 @@ func TestDocument(t *testing.T) { testhelper.PrintMemStats() assert.Equal(t, textSize, doc.GarbageLen()) + // 03. GC assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) runtime.GC() fmt.Println("-----Garbage collect") @@ -251,7 +288,7 @@ func TestDocument(t *testing.T) { testhelper.PrintMemStats() assert.NoError(t, err) - // 03. long text by one node + // 04. long text by one node err = doc.Update(func(root *proxy.ObjectProxy) error { text := root.SetNewText("k2") str := "" @@ -265,7 +302,7 @@ func TestDocument(t *testing.T) { testhelper.PrintBytesSize(bytes) testhelper.PrintMemStats() - // 04. Modify one node multiple times + // 05. Modify one node multiple times err = doc.Update(func(root *proxy.ObjectProxy) error { text := root.GetText("k2") for i := 0; i < textSize; i++ { @@ -281,6 +318,7 @@ func TestDocument(t *testing.T) { testhelper.PrintBytesSize(bytes) testhelper.PrintMemStats() + // 06. GC assert.Equal(t, textSize, doc.GarbageLen()) assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) runtime.GC() diff --git a/pkg/document/json/rich_text.go b/pkg/document/json/rich_text.go index f4842fa93..5bc5e80b9 100644 --- a/pkg/document/json/rich_text.go +++ b/pkg/document/json/rich_text.go @@ -256,3 +256,14 @@ func (t *RichText) Nodes() []*RGATreeSplitNode { func (t *RichText) AnnotatedString() string { return t.rgaTreeSplit.AnnotatedString() } + +// removedNodesLen returns length of removed nodes +func (t *RichText) removedNodesLen() int { + return t.rgaTreeSplit.removedNodesLen() +} + +// cleanupRemovedNodes cleans up nodes that have been removed. +// The cleaned nodes are subject to garbage collector collection. +func (t *RichText) cleanupRemovedNodes(ticket *time.Ticket) int { + return t.rgaTreeSplit.cleanupRemovedNodes(ticket) +} diff --git a/pkg/document/operation/rich_edit.go b/pkg/document/operation/rich_edit.go index 9245dbe03..d57a8fa64 100644 --- a/pkg/document/operation/rich_edit.go +++ b/pkg/document/operation/rich_edit.go @@ -57,6 +57,7 @@ func (e *RichEdit) Execute(root *json.Root) error { switch obj := parent.(type) { case *json.RichText: obj.Edit(e.from, e.to, e.latestCreatedAtMapByActor, e.content, e.attributes, e.executedAt) + root.RegisterEditedTextElement(obj) default: return ErrNotApplicableDataType } diff --git a/pkg/document/proxy/rich_text_proxy.go b/pkg/document/proxy/rich_text_proxy.go index 5e562b8f3..dfa202b6b 100644 --- a/pkg/document/proxy/rich_text_proxy.go +++ b/pkg/document/proxy/rich_text_proxy.go @@ -64,6 +64,7 @@ func (p *RichTextProxy) Edit(from, to int, content string, attributes map[string attributes, ticket, )) + p.context.RegisterEditedTextElement(p) return p } From c3e72771834213b41d46118c193f953d4255eabc Mon Sep 17 00:00:00 2001 From: dc7303 Date: Tue, 17 Nov 2020 23:03:54 +0900 Subject: [PATCH 03/14] Add client test for GC Garbage collection synchronization test added for text type elements. This is because garbage collection information must be synchronized between different clients. --- client/client_test.go | 93 +++++++++++++++++++++++++++++++++++---- pkg/document/json/root.go | 7 ++- 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index a6ca5c036..c7ab2a284 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -806,7 +806,7 @@ func TestClientAndDocument(t *testing.T) { assert.Equal(t, d1.Marshal(), d2.Marshal()) }) - t.Run("garbage collection test", func(t *testing.T) { + t.Run("garbage collection for container type test", func(t *testing.T) { ctx := context.Background() d1 := document.New(testhelper.Collection, t.Name()) @@ -874,6 +874,79 @@ func TestClientAndDocument(t *testing.T) { assert.Equal(t, 0, d2.GarbageLen()) }) + t.Run("garbage collection for text type test", func(t *testing.T) { + ctx := context.Background() + + d1 := document.New(testhelper.Collection, t.Name()) + err := c1.Attach(ctx, d1) + assert.NoError(t, err) + + d2 := document.New(testhelper.Collection, t.Name()) + err = c2.Attach(ctx, d2) + assert.NoError(t, err) + + err = d1.Update(func(root *proxy.ObjectProxy) error { + root.SetNewText("text"). + Edit(0, 0, "Hello world") + root.SetNewRichText("richText"). + Edit(0, 0, "Hello world", nil) + return nil + }, "sets test and richText") + assert.NoError(t, err) + assert.Equal(t, 0, d1.GarbageLen()) + assert.Equal(t, 0, d2.GarbageLen()) + + // (0, 0) -> (1, 0): syncedseqs:(0, 0) + err = c1.Sync(ctx) + assert.NoError(t, err) + + // (1, 0) -> (1, 1): syncedseqs:(0, 0) + err = c2.Sync(ctx) + assert.NoError(t, err) + + err = d2.Update(func(root *proxy.ObjectProxy) error { + root.GetText("text"). + Edit(0, 1, "a"). + Edit(1, 2, "b") + root.GetRichText("richText"). + Edit(0, 1, "a", map[string]string{"b": "1"}) + return nil + }, "edit text type elements") + assert.NoError(t, err) + assert.Equal(t, 0, d1.GarbageLen()) + assert.Equal(t, 3, d2.GarbageLen()) + + // (1, 1) -> (1, 2): syncedseqs:(0, 1) + err = c2.Sync(ctx) + assert.NoError(t, err) + assert.Equal(t, 0, d1.GarbageLen()) + assert.Equal(t, 3, d2.GarbageLen()) + + // (1, 2) -> (2, 2): syncedseqs:(1, 1) + err = c1.Sync(ctx) + assert.NoError(t, err) + assert.Equal(t, 3, d1.GarbageLen()) + assert.Equal(t, 3, d2.GarbageLen()) + + // (2, 2) -> (2, 2): syncedseqs:(1, 2) + err = c2.Sync(ctx) + assert.NoError(t, err) + assert.Equal(t, 3, d1.GarbageLen()) + assert.Equal(t, 3, d2.GarbageLen()) + + // (2, 2) -> (2, 2): syncedseqs:(2, 2): meet GC condition + err = c1.Sync(ctx) + assert.NoError(t, err) + assert.Equal(t, 0, d1.GarbageLen()) + assert.Equal(t, 3, d2.GarbageLen()) + + // (2, 2) -> (2, 2): syncedseqs:(2, 2): meet GC condition + err = c2.Sync(ctx) + assert.NoError(t, err) + assert.Equal(t, 0, d1.GarbageLen()) + assert.Equal(t, 0, d2.GarbageLen()) + }) + t.Run("garbage collection with detached document test", func(t *testing.T) { ctx := context.Background() @@ -889,8 +962,10 @@ func TestClientAndDocument(t *testing.T) { root.SetInteger("1", 1) root.SetNewArray("2").AddInteger(1, 2, 3) root.SetInteger("3", 3) + root.SetNewText("4").Edit(0, 0, "Hi") + root.SetNewRichText("5").Edit(0, 0, "Hi", nil) return nil - }, "sets 1,2,3") + }, "sets 1,2,3,4,5") assert.NoError(t, err) assert.Equal(t, 0, d1.GarbageLen()) assert.Equal(t, 0, d2.GarbageLen()) @@ -905,16 +980,18 @@ func TestClientAndDocument(t *testing.T) { err = d1.Update(func(root *proxy.ObjectProxy) error { root.Delete("2") + root.GetText("4").Edit(0, 1, "h") + root.GetRichText("5").Edit(0, 1, "h", map[string]string{"b": "1"}) return nil - }, "removes 2") + }, "removes 2 and edit text type elements") assert.NoError(t, err) - assert.Equal(t, 4, d1.GarbageLen()) + assert.Equal(t, 6, d1.GarbageLen()) assert.Equal(t, 0, d2.GarbageLen()) // (1, 1) -> (2, 1): syncedseqs:(1, 0) err = c1.Sync(ctx) assert.NoError(t, err) - assert.Equal(t, 4, d1.GarbageLen()) + assert.Equal(t, 6, d1.GarbageLen()) assert.Equal(t, 0, d2.GarbageLen()) err = c2.Detach(ctx, d2) @@ -923,14 +1000,14 @@ func TestClientAndDocument(t *testing.T) { // (2, 1) -> (2, 2): syncedseqs:(1, x) err = c2.Sync(ctx) assert.NoError(t, err) - assert.Equal(t, 4, d1.GarbageLen()) - assert.Equal(t, 4, d2.GarbageLen()) + assert.Equal(t, 6, d1.GarbageLen()) + assert.Equal(t, 6, d2.GarbageLen()) // (2, 2) -> (2, 2): syncedseqs:(2, x): meet GC condition err = c1.Sync(ctx) assert.NoError(t, err) assert.Equal(t, 0, d1.GarbageLen()) - assert.Equal(t, 4, d2.GarbageLen()) + assert.Equal(t, 6, d2.GarbageLen()) }) } diff --git a/pkg/document/json/root.go b/pkg/document/json/root.go index e3c667274..81ac140e5 100644 --- a/pkg/document/json/root.go +++ b/pkg/document/json/root.go @@ -109,8 +109,11 @@ func (r *Root) GarbageCollect(ticket *time.Ticket) int { } for _, text := range r.editedTextElementMapByCreatedAt { - count += text.cleanupRemovedNodes(ticket) - delete(r.editedTextElementMapByCreatedAt, text.CreatedAt().Key()) + removedNodeCnt := text.cleanupRemovedNodes(ticket) + if removedNodeCnt > 0 { + delete(r.editedTextElementMapByCreatedAt, text.CreatedAt().Key()) + } + count += removedNodeCnt } return count From 6d88de709b3780d6d1bc9a307c0140adc6ad62d0 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Tue, 17 Nov 2020 23:22:49 +0900 Subject: [PATCH 04/14] Change removedNodeMap to removedNodesMap --- pkg/document/json/rga_tree_split.go | 42 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/pkg/document/json/rga_tree_split.go b/pkg/document/json/rga_tree_split.go index d8479d806..8dcc244a7 100644 --- a/pkg/document/json/rga_tree_split.go +++ b/pkg/document/json/rga_tree_split.go @@ -222,14 +222,14 @@ func (t *RGATreeSplitNode) Value() RGATreeSplitValue { return t.value } -// removedNodeMapKey is a key of removedNodeMap. +// removedNodesMapKey is a key of removedNodesMap. // The shape of the key is '{createdAt.Key()}:{offset}'. -type removedNodeMapKey string +type removedNodesMapKey string -// createRemovedNodeMapKey generates keys for removedNodes. -func createRemovedNodeMapKey(createdAtKey string, offset int) removedNodeMapKey { +// createRemovedNodesMapKey generates keys for removedNodes. +func createRemovedNodesMapKey(createdAtKey string, offset int) removedNodesMapKey { key := fmt.Sprintf("%s:%d", createdAtKey, offset) - return removedNodeMapKey(key) + return removedNodesMapKey(key) } type RGATreeSplit struct { @@ -237,7 +237,7 @@ type RGATreeSplit struct { treeByIndex *splay.Tree treeByID *llrb.Tree - removedNodeMap map[removedNodeMapKey]*RGATreeSplitNode + removedNodesMap map[removedNodesMapKey]*RGATreeSplitNode } func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { @@ -246,10 +246,10 @@ func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { treeByID.Put(initialHead.ID(), initialHead) return &RGATreeSplit{ - initialHead: initialHead, - treeByIndex: treeByIndex, - treeByID: treeByID, - removedNodeMap: make(map[removedNodeMapKey]*RGATreeSplitNode), + initialHead: initialHead, + treeByIndex: treeByIndex, + treeByID: treeByID, + removedNodesMap: make(map[removedNodesMapKey]*RGATreeSplitNode), } } @@ -386,7 +386,7 @@ func (s *RGATreeSplit) edit( // 02. delete between from and to nodesToDelete := s.findBetween(fromRight, toRight) - latestCreatedAtMap, removedNodeMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) + latestCreatedAtMap, removedNodesMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) var caretID *RGATreeSplitNodeID if toRight == nil { @@ -403,8 +403,8 @@ func (s *RGATreeSplit) edit( } // 04. add removed node - for key, removedNode := range removedNodeMapByCreatedAt { - s.removedNodeMap[key] = removedNode + for key, removedNode := range removedNodesMapByCreatedAt { + s.removedNodesMap[key] = removedNode } return caretPos, latestCreatedAtMap @@ -424,9 +424,9 @@ func (s *RGATreeSplit) deleteNodes( candidates []*RGATreeSplitNode, latestCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, -) (map[string]*time.Ticket, map[removedNodeMapKey]*RGATreeSplitNode) { +) (map[string]*time.Ticket, map[removedNodesMapKey]*RGATreeSplitNode) { createdAtMapByActor := make(map[string]*time.Ticket) - removedNodeMap := make(map[removedNodeMapKey]*RGATreeSplitNode) + removedNodesMap := make(map[removedNodesMapKey]*RGATreeSplitNode) for _, node := range candidates { actorIDHex := node.createdAt().ActorIDHex() @@ -451,12 +451,12 @@ func (s *RGATreeSplit) deleteNodes( createdAtMapByActor[actorIDHex] = createdAt } - key := createRemovedNodeMapKey(createdAt.Key(), node.id.offset) - removedNodeMap[key] = node + key := createRemovedNodesMapKey(createdAt.Key(), node.id.offset) + removedNodesMap[key] = node } } - return createdAtMapByActor, removedNodeMap + return createdAtMapByActor, removedNodesMap } func (s *RGATreeSplit) marshal() string { @@ -516,19 +516,19 @@ func (s *RGATreeSplit) AnnotatedString() string { // removedNodesLen returns length of removed nodes func (s *RGATreeSplit) removedNodesLen() int { - return len(s.removedNodeMap) + return len(s.removedNodesMap) } // cleanupRemovedNodes cleans up nodes that have been removed. // The cleaned nodes are subject to garbage collector collection. func (s *RGATreeSplit) cleanupRemovedNodes(ticket *time.Ticket) int { count := 0 - for _, node := range s.removedNodeMap { + for _, node := range s.removedNodesMap { if node.removedAt != nil && ticket.Compare(node.removedAt) >= 0 { s.treeByIndex.Delete(node.indexNode) s.purge(node) s.treeByID.Remove(node.id) - delete(s.removedNodeMap, removedNodeMapKey(node.createdAt().Key())) + delete(s.removedNodesMap, removedNodesMapKey(node.createdAt().Key())) count++ } } From 5feaf71ed38ab5c543231af2f7b5a32667d41898 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Tue, 17 Nov 2020 23:26:57 +0900 Subject: [PATCH 05/14] Fix textSize variable in large size of text garbage test --- pkg/document/document_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 2ec8f7673..02fd2ec12 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -249,7 +249,7 @@ func TestDocument(t *testing.T) { assert.Equal(t, "{}", doc.Marshal()) assert.False(t, doc.HasLocalChanges()) - textSize := 10 + textSize := 1000 // 01. initial err := doc.Update(func(root *proxy.ObjectProxy) error { text := root.SetNewText("k1") From 70afac92d055b244a173b9785e17ed1f85204985 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 22 Nov 2020 14:36:00 +0900 Subject: [PATCH 06/14] Change removedNodesMap to removedNodeMap --- pkg/document/json/rga_tree_split.go | 42 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/pkg/document/json/rga_tree_split.go b/pkg/document/json/rga_tree_split.go index 8dcc244a7..d8479d806 100644 --- a/pkg/document/json/rga_tree_split.go +++ b/pkg/document/json/rga_tree_split.go @@ -222,14 +222,14 @@ func (t *RGATreeSplitNode) Value() RGATreeSplitValue { return t.value } -// removedNodesMapKey is a key of removedNodesMap. +// removedNodeMapKey is a key of removedNodeMap. // The shape of the key is '{createdAt.Key()}:{offset}'. -type removedNodesMapKey string +type removedNodeMapKey string -// createRemovedNodesMapKey generates keys for removedNodes. -func createRemovedNodesMapKey(createdAtKey string, offset int) removedNodesMapKey { +// createRemovedNodeMapKey generates keys for removedNodes. +func createRemovedNodeMapKey(createdAtKey string, offset int) removedNodeMapKey { key := fmt.Sprintf("%s:%d", createdAtKey, offset) - return removedNodesMapKey(key) + return removedNodeMapKey(key) } type RGATreeSplit struct { @@ -237,7 +237,7 @@ type RGATreeSplit struct { treeByIndex *splay.Tree treeByID *llrb.Tree - removedNodesMap map[removedNodesMapKey]*RGATreeSplitNode + removedNodeMap map[removedNodeMapKey]*RGATreeSplitNode } func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { @@ -246,10 +246,10 @@ func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { treeByID.Put(initialHead.ID(), initialHead) return &RGATreeSplit{ - initialHead: initialHead, - treeByIndex: treeByIndex, - treeByID: treeByID, - removedNodesMap: make(map[removedNodesMapKey]*RGATreeSplitNode), + initialHead: initialHead, + treeByIndex: treeByIndex, + treeByID: treeByID, + removedNodeMap: make(map[removedNodeMapKey]*RGATreeSplitNode), } } @@ -386,7 +386,7 @@ func (s *RGATreeSplit) edit( // 02. delete between from and to nodesToDelete := s.findBetween(fromRight, toRight) - latestCreatedAtMap, removedNodesMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) + latestCreatedAtMap, removedNodeMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) var caretID *RGATreeSplitNodeID if toRight == nil { @@ -403,8 +403,8 @@ func (s *RGATreeSplit) edit( } // 04. add removed node - for key, removedNode := range removedNodesMapByCreatedAt { - s.removedNodesMap[key] = removedNode + for key, removedNode := range removedNodeMapByCreatedAt { + s.removedNodeMap[key] = removedNode } return caretPos, latestCreatedAtMap @@ -424,9 +424,9 @@ func (s *RGATreeSplit) deleteNodes( candidates []*RGATreeSplitNode, latestCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, -) (map[string]*time.Ticket, map[removedNodesMapKey]*RGATreeSplitNode) { +) (map[string]*time.Ticket, map[removedNodeMapKey]*RGATreeSplitNode) { createdAtMapByActor := make(map[string]*time.Ticket) - removedNodesMap := make(map[removedNodesMapKey]*RGATreeSplitNode) + removedNodeMap := make(map[removedNodeMapKey]*RGATreeSplitNode) for _, node := range candidates { actorIDHex := node.createdAt().ActorIDHex() @@ -451,12 +451,12 @@ func (s *RGATreeSplit) deleteNodes( createdAtMapByActor[actorIDHex] = createdAt } - key := createRemovedNodesMapKey(createdAt.Key(), node.id.offset) - removedNodesMap[key] = node + key := createRemovedNodeMapKey(createdAt.Key(), node.id.offset) + removedNodeMap[key] = node } } - return createdAtMapByActor, removedNodesMap + return createdAtMapByActor, removedNodeMap } func (s *RGATreeSplit) marshal() string { @@ -516,19 +516,19 @@ func (s *RGATreeSplit) AnnotatedString() string { // removedNodesLen returns length of removed nodes func (s *RGATreeSplit) removedNodesLen() int { - return len(s.removedNodesMap) + return len(s.removedNodeMap) } // cleanupRemovedNodes cleans up nodes that have been removed. // The cleaned nodes are subject to garbage collector collection. func (s *RGATreeSplit) cleanupRemovedNodes(ticket *time.Ticket) int { count := 0 - for _, node := range s.removedNodesMap { + for _, node := range s.removedNodeMap { if node.removedAt != nil && ticket.Compare(node.removedAt) >= 0 { s.treeByIndex.Delete(node.indexNode) s.purge(node) s.treeByID.Remove(node.id) - delete(s.removedNodesMap, removedNodesMapKey(node.createdAt().Key())) + delete(s.removedNodeMap, removedNodeMapKey(node.createdAt().Key())) count++ } } From 65cc0ab823827196f5a3b25480399466c7687a49 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 22 Nov 2020 14:39:25 +0900 Subject: [PATCH 07/14] Fix group the package in converter_test We hope to group the package into 3 parts. Standard library, 3rd party library, internal package. --- api/converter/converter_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/converter/converter_test.go b/api/converter/converter_test.go index bcfb1998c..bc7509ba2 100644 --- a/api/converter/converter_test.go +++ b/api/converter/converter_test.go @@ -21,13 +21,12 @@ import ( "testing" "time" - time2 "github.com/yorkie-team/yorkie/pkg/document/time" - "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/api/converter" "github.com/yorkie-team/yorkie/pkg/document" "github.com/yorkie-team/yorkie/pkg/document/proxy" + time2 "github.com/yorkie-team/yorkie/pkg/document/time" ) func TestConverter(t *testing.T) { From d16bfc226a28fa00e78b02f6e472c753f23fe292 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 22 Nov 2020 16:13:51 +0900 Subject: [PATCH 08/14] Fix editedTextElementMapByCreatedAt property Change editedTextElementMapByCreatedAt to removedNodeTextElementMapByCreatedAt. This is to cache only when deleted nodes exist. For this, this feature compares the offset of RGATreeSpltPos. --- pkg/document/change/context.go | 4 ++-- pkg/document/json/rga_tree_split.go | 11 +++++++++++ pkg/document/json/root.go | 26 +++++++++++++------------- pkg/document/operation/edit.go | 4 +++- pkg/document/operation/rich_edit.go | 4 +++- pkg/document/proxy/rich_text_proxy.go | 4 +++- pkg/document/proxy/text_proxy.go | 5 +++-- 7 files changed, 38 insertions(+), 20 deletions(-) diff --git a/pkg/document/change/context.go b/pkg/document/change/context.go index c75fc6329..82dd319c5 100644 --- a/pkg/document/change/context.go +++ b/pkg/document/change/context.go @@ -78,6 +78,6 @@ func (c *Context) RegisterRemovedElementPair(parent json.Container, deleted json c.root.RegisterRemovedElementPair(parent, deleted) } -func (c *Context) RegisterEditedTextElement(textType json.TextElement) { - c.root.RegisterEditedTextElement(textType) +func (c *Context) RegisterRemovedNodeTextElement(textType json.TextElement) { + c.root.RegisterRemovedNodeTextElement(textType) } diff --git a/pkg/document/json/rga_tree_split.go b/pkg/document/json/rga_tree_split.go index d8479d806..cf7c5e367 100644 --- a/pkg/document/json/rga_tree_split.go +++ b/pkg/document/json/rga_tree_split.go @@ -103,6 +103,17 @@ func (pos *RGATreeSplitNodePos) RelativeOffset() int { return pos.relativeOffset } +// Compare compares the offset between RGATreeSplitNodePos. +func (pos *RGATreeSplitNodePos) Compare(other *RGATreeSplitNodePos) int { + if pos.relativeOffset > other.relativeOffset { + return 1 + } else if pos.relativeOffset < other.relativeOffset { + return -1 + } + + return 0 +} + type Selection struct { from *RGATreeSplitNodePos to *RGATreeSplitNodePos diff --git a/pkg/document/json/root.go b/pkg/document/json/root.go index 81ac140e5..c6054143d 100644 --- a/pkg/document/json/root.go +++ b/pkg/document/json/root.go @@ -32,18 +32,18 @@ type ElementPair struct { // Every element has a unique time ticket at creation, which allows us to find // a particular element. type Root struct { - object *Object - elementMapByCreatedAt map[string]Element - removedElementPairMapByCreatedAt map[string]ElementPair - editedTextElementMapByCreatedAt map[string]TextElement + object *Object + elementMapByCreatedAt map[string]Element + removedElementPairMapByCreatedAt map[string]ElementPair + removedNodeTextElementMapByCreatedAt map[string]TextElement } // NewRoot creates a new instance of Root. func NewRoot(root *Object) *Root { r := &Root{ - elementMapByCreatedAt: make(map[string]Element), - removedElementPairMapByCreatedAt: make(map[string]ElementPair), - editedTextElementMapByCreatedAt: make(map[string]TextElement), + elementMapByCreatedAt: make(map[string]Element), + removedElementPairMapByCreatedAt: make(map[string]ElementPair), + removedNodeTextElementMapByCreatedAt: make(map[string]TextElement), } r.object = root @@ -87,9 +87,9 @@ func (r *Root) RegisterRemovedElementPair(parent Container, elem Element) { } } -// RegisterEditedTextElement register the given text element to hash table. -func (r *Root) RegisterEditedTextElement(textType TextElement) { - r.editedTextElementMapByCreatedAt[textType.CreatedAt().Key()] = textType +// RegisterRemovedNodeTextElement register the given text element to hash table. +func (r *Root) RegisterRemovedNodeTextElement(textType TextElement) { + r.removedNodeTextElementMapByCreatedAt[textType.CreatedAt().Key()] = textType } // DeepCopy copies itself deeply. @@ -108,10 +108,10 @@ func (r *Root) GarbageCollect(ticket *time.Ticket) int { } } - for _, text := range r.editedTextElementMapByCreatedAt { + for _, text := range r.removedNodeTextElementMapByCreatedAt { removedNodeCnt := text.cleanupRemovedNodes(ticket) if removedNodeCnt > 0 { - delete(r.editedTextElementMapByCreatedAt, text.CreatedAt().Key()) + delete(r.removedNodeTextElementMapByCreatedAt, text.CreatedAt().Key()) } count += removedNodeCnt } @@ -135,7 +135,7 @@ func (r *Root) GarbageLen() int { } } - for _, text := range r.editedTextElementMapByCreatedAt { + for _, text := range r.removedNodeTextElementMapByCreatedAt { count += text.removedNodesLen() } diff --git a/pkg/document/operation/edit.go b/pkg/document/operation/edit.go index c45ee362a..53f6c755f 100644 --- a/pkg/document/operation/edit.go +++ b/pkg/document/operation/edit.go @@ -54,7 +54,9 @@ func (e *Edit) Execute(root *json.Root) error { switch obj := parent.(type) { case *json.Text: obj.Edit(e.from, e.to, e.latestCreatedAtMapByActor, e.content, e.executedAt) - root.RegisterEditedTextElement(obj) + if e.from.Compare(e.to) != 0 { + root.RegisterRemovedNodeTextElement(obj) + } default: return ErrNotApplicableDataType } diff --git a/pkg/document/operation/rich_edit.go b/pkg/document/operation/rich_edit.go index d57a8fa64..da7613a3b 100644 --- a/pkg/document/operation/rich_edit.go +++ b/pkg/document/operation/rich_edit.go @@ -57,7 +57,9 @@ func (e *RichEdit) Execute(root *json.Root) error { switch obj := parent.(type) { case *json.RichText: obj.Edit(e.from, e.to, e.latestCreatedAtMapByActor, e.content, e.attributes, e.executedAt) - root.RegisterEditedTextElement(obj) + if e.from.Compare(e.to) != 0 { + root.RegisterRemovedNodeTextElement(obj) + } default: return ErrNotApplicableDataType } diff --git a/pkg/document/proxy/rich_text_proxy.go b/pkg/document/proxy/rich_text_proxy.go index dfa202b6b..287bc635e 100644 --- a/pkg/document/proxy/rich_text_proxy.go +++ b/pkg/document/proxy/rich_text_proxy.go @@ -64,7 +64,9 @@ func (p *RichTextProxy) Edit(from, to int, content string, attributes map[string attributes, ticket, )) - p.context.RegisterEditedTextElement(p) + if fromPos.Compare(toPos) != 0 { + p.context.RegisterRemovedNodeTextElement(p) + } return p } diff --git a/pkg/document/proxy/text_proxy.go b/pkg/document/proxy/text_proxy.go index f19a05ac6..3b02dcb08 100644 --- a/pkg/document/proxy/text_proxy.go +++ b/pkg/document/proxy/text_proxy.go @@ -62,7 +62,8 @@ func (p *TextProxy) Edit(from, to int, content string) *TextProxy { content, ticket, )) - p.context.RegisterEditedTextElement(p) - + if fromPos.Compare(toPos) != 0 { + p.context.RegisterRemovedNodeTextElement(p) + } return p } From 18a8ae4dba0186be52946b9539009df8c83e55a9 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sat, 19 Dec 2020 12:22:08 +0900 Subject: [PATCH 09/14] Fix test case to output memory status Corrected because the size of the object could not be output properly. --- pkg/document/document_test.go | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 02fd2ec12..6a2313156 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -22,6 +22,8 @@ import ( "runtime" "testing" + "github.com/yorkie-team/yorkie/pkg/document/json" + "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/api/converter" @@ -249,7 +251,14 @@ func TestDocument(t *testing.T) { assert.Equal(t, "{}", doc.Marshal()) assert.False(t, doc.HasLocalChanges()) - textSize := 1000 + printMemStats := func(root *json.Object) { + bytes, err := converter.ObjectToBytes(doc.RootObject()) + assert.NoError(t, err) + testhelper.PrintBytesSize(bytes) + testhelper.PrintMemStats() + } + + textSize := 1_000 // 01. initial err := doc.Update(func(root *proxy.ObjectProxy) error { text := root.SetNewText("k1") @@ -259,11 +268,8 @@ func TestDocument(t *testing.T) { return nil }, "initial") assert.NoError(t, err) - bytes, err := converter.ObjectToBytes(doc.RootObject()) - assert.NoError(t, err) fmt.Println("-----initial") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() + printMemStats(doc.RootObject()) // 02. 1000 nodes modified err = doc.Update(func(root *proxy.ObjectProxy) error { @@ -274,19 +280,15 @@ func TestDocument(t *testing.T) { return nil }, "1000 nodes modified") assert.NoError(t, err) - assert.NoError(t, err) fmt.Println("-----1000 nodes modified") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() + printMemStats(doc.RootObject()) assert.Equal(t, textSize, doc.GarbageLen()) // 03. GC assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) runtime.GC() fmt.Println("-----Garbage collect") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() - assert.NoError(t, err) + printMemStats(doc.RootObject()) // 04. long text by one node err = doc.Update(func(root *proxy.ObjectProxy) error { @@ -299,8 +301,7 @@ func TestDocument(t *testing.T) { return nil }, "initial") fmt.Println("-----long text by one node") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() + printMemStats(doc.RootObject()) // 05. Modify one node multiple times err = doc.Update(func(root *proxy.ObjectProxy) error { @@ -313,18 +314,15 @@ func TestDocument(t *testing.T) { return nil }, "Modify one node multiple times") assert.NoError(t, err) - assert.NoError(t, err) fmt.Println("-----Modify one node multiple times") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() + printMemStats(doc.RootObject()) // 06. GC assert.Equal(t, textSize, doc.GarbageLen()) assert.Equal(t, textSize, doc.GarbageCollect(time.MaxTicket)) runtime.GC() fmt.Println("-----Garbage collect") - testhelper.PrintBytesSize(bytes) - testhelper.PrintMemStats() + printMemStats(doc.RootObject()) }) t.Run("object test", func(t *testing.T) { From 5ce761c8b01c754df4262f308bafa8f66ca4469e Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sat, 19 Dec 2020 17:40:48 +0900 Subject: [PATCH 10/14] Add garbage collect test for TextElement The coverage measurement method of 'Go cover' is basically only measured within the package being tested. So I added 'root_test.go' and wrote test code in it to increase the coverage score. --- pkg/document/json/array_test.go | 3 +- pkg/document/json/object_test.go | 3 +- pkg/document/json/rich_text_test.go | 3 +- pkg/document/json/root_test.go | 105 ++++++++++++++++++++++++++++ pkg/document/json/text_test.go | 3 +- testhelper/testhelper.go | 8 ++- 6 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 pkg/document/json/root_test.go diff --git a/pkg/document/json/array_test.go b/pkg/document/json/array_test.go index f98994b5d..84e781d3d 100644 --- a/pkg/document/json/array_test.go +++ b/pkg/document/json/array_test.go @@ -27,7 +27,8 @@ import ( func TestArray(t *testing.T) { t.Run("marshal test", func(t *testing.T) { - ctx := testhelper.TextChangeContext() + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) a := json.NewArray(json.NewRGATreeList(), ctx.IssueTimeTicket()) diff --git a/pkg/document/json/object_test.go b/pkg/document/json/object_test.go index e57e0d9d6..2d1015330 100644 --- a/pkg/document/json/object_test.go +++ b/pkg/document/json/object_test.go @@ -11,7 +11,8 @@ import ( func TestObject(t *testing.T) { t.Run("marshal test", func(t *testing.T) { - ctx := testhelper.TextChangeContext() + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) obj := json.NewObject(json.NewRHTPriorityQueueMap(), ctx.IssueTimeTicket()) diff --git a/pkg/document/json/rich_text_test.go b/pkg/document/json/rich_text_test.go index 1a17cb9e6..458381441 100644 --- a/pkg/document/json/rich_text_test.go +++ b/pkg/document/json/rich_text_test.go @@ -27,7 +27,8 @@ import ( func TestRichText(t *testing.T) { t.Run("marshal test", func(t *testing.T) { - ctx := testhelper.TextChangeContext() + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) text := json.NewInitialRichText(json.NewRGATreeSplit(json.InitialRichTextNode()), ctx.IssueTimeTicket()) diff --git a/pkg/document/json/root_test.go b/pkg/document/json/root_test.go new file mode 100644 index 000000000..c00c158e2 --- /dev/null +++ b/pkg/document/json/root_test.go @@ -0,0 +1,105 @@ +/* + * Copyright 2020 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package json_test + +import ( + "testing" + + "github.com/yorkie-team/yorkie/pkg/document/time" + + "github.com/stretchr/testify/assert" + + "github.com/yorkie-team/yorkie/pkg/document/json" + "github.com/yorkie-team/yorkie/testhelper" +) + +func registerTextElement(fromPos, toPos *json.RGATreeSplitNodePos, root *json.Root, text json.TextElement) { + if fromPos.Compare(toPos) != 0 { + root.RegisterRemovedNodeTextElement(text) + } +} + +func TestRoot(t *testing.T) { + t.Run("garbage collect for text test", func(t *testing.T) { + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) + text := json.NewText(json.NewRGATreeSplit(json.InitialTextNode()), ctx.IssueTimeTicket()) + + fromPos, toPos := text.CreateRange(0, 0) + text.Edit(fromPos, toPos, nil, "Hello World", ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, text) + assert.Equal(t, `"Hello World"`, text.Marshal()) + assert.Equal(t, 0, root.GarbageLen()) + + fromPos, toPos = text.CreateRange(6, 11) + text.Edit(fromPos, toPos, nil, "Yorkie", ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, text) + assert.Equal(t, `"Hello Yorkie"`, text.Marshal()) + assert.Equal(t, 1, root.GarbageLen()) + + fromPos, toPos = text.CreateRange(0, 6) + text.Edit(fromPos, toPos, nil, "", ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, text) + assert.Equal(t, `"Yorkie"`, text.Marshal()) + assert.Equal(t, 2, root.GarbageLen()) + + // It contains code marked tombstone. + // After calling the garbage collector, the node will be removed. + nodeLen := len(text.Nodes()) + assert.Equal(t, 3, nodeLen) + + assert.Equal(t, 2, root.GarbageCollect(time.MaxTicket)) + assert.Equal(t, 0, root.GarbageLen()) + nodeLen = len(text.Nodes()) + assert.Equal(t, 1, nodeLen) + }) + + t.Run("garbage collect for rich text test", func(t *testing.T) { + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) + richText := json.NewRichText(json.NewRGATreeSplit(json.InitialTextNode()), ctx.IssueTimeTicket()) + + fromPos, toPos := richText.CreateRange(0, 0) + richText.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, richText) + assert.Equal(t, `[{"attrs":{},"val":"Hello World"}]`, richText.Marshal()) + assert.Equal(t, 0, root.GarbageLen()) + + fromPos, toPos = richText.CreateRange(6, 11) + richText.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, richText) + assert.Equal(t, `[{"attrs":{},"val":"Hello "},{"attrs":{},"val":"Yorkie"}]`, richText.Marshal()) + assert.Equal(t, 1, root.GarbageLen()) + + fromPos, toPos = richText.CreateRange(0, 6) + richText.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket()) + registerTextElement(fromPos, toPos, root, richText) + assert.Equal(t, `[{"attrs":{},"val":"Yorkie"}]`, richText.Marshal()) + assert.Equal(t, 2, root.GarbageLen()) + + // It contains code marked tombstone. + // After calling the garbage collector, the node will be removed. + nodeLen := len(richText.Nodes()) + assert.Equal(t, 3, nodeLen) + + assert.Equal(t, 2, root.GarbageCollect(time.MaxTicket)) + assert.Equal(t, 0, root.GarbageLen()) + + nodeLen = len(richText.Nodes()) + assert.Equal(t, 1, nodeLen) + }) +} diff --git a/pkg/document/json/text_test.go b/pkg/document/json/text_test.go index 4bf987220..8bafd1465 100644 --- a/pkg/document/json/text_test.go +++ b/pkg/document/json/text_test.go @@ -27,7 +27,8 @@ import ( func TestText(t *testing.T) { t.Run("marshal test", func(t *testing.T) { - ctx := testhelper.TextChangeContext() + root := testhelper.TestRoot() + ctx := testhelper.TextChangeContext(root) text := json.NewText(json.NewRGATreeSplit(json.InitialTextNode()), ctx.IssueTimeTicket()) fromPos, toPos := text.CreateRange(0, 0) diff --git a/testhelper/testhelper.go b/testhelper/testhelper.go index 907a9512b..e762e42c0 100644 --- a/testhelper/testhelper.go +++ b/testhelper/testhelper.go @@ -58,12 +58,16 @@ func TestDBName() string { return fmt.Sprintf("test-%s-%d", yorkie.DefaultMongoYorkieDatabase, testStartedAt) } +func TestRoot() *json.Root { + return json.NewRoot(json.NewObject(json.NewRHTPriorityQueueMap(), time.InitialTicket)) +} + // TextChangeContext returns the context of test change. -func TextChangeContext() *change.Context { +func TextChangeContext(root *json.Root) *change.Context { return change.NewContext( change.InitialID, "", - json.NewRoot(json.NewObject(json.NewRHTPriorityQueueMap(), time.InitialTicket)), + root, ) } From b52d78d2b15b9197ae90a359e0d43199677d5a36 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 20 Dec 2020 18:15:18 +0900 Subject: [PATCH 11/14] Cleanup import code syntax --- api/converter/converter_test.go | 14 +++++++------- pkg/document/document_test.go | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/api/converter/converter_test.go b/api/converter/converter_test.go index bc7509ba2..9ad62b066 100644 --- a/api/converter/converter_test.go +++ b/api/converter/converter_test.go @@ -19,14 +19,14 @@ package converter_test import ( "math" "testing" - "time" + gotime "time" "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/api/converter" "github.com/yorkie-team/yorkie/pkg/document" "github.com/yorkie-team/yorkie/pkg/document/proxy" - time2 "github.com/yorkie-team/yorkie/pkg/document/time" + "github.com/yorkie-team/yorkie/pkg/document/time" ) func TestConverter(t *testing.T) { @@ -67,7 +67,7 @@ func TestConverter(t *testing.T) { SetDouble("1.4", 1.79). SetString("k1.5", "4"). SetBytes("k1.6", []byte{65, 66}). - SetDate("k1.7", time.Now()) + SetDate("k1.7", gotime.Now()) // an array root.SetNewArray("k2"). @@ -77,7 +77,7 @@ func TestConverter(t *testing.T) { AddDouble(3.0). AddString("4"). AddBytes([]byte{65}). - AddDate(time.Now()) + AddDate(gotime.Now()) // plain text root.SetNewText("k3"). @@ -122,7 +122,7 @@ func TestConverter(t *testing.T) { SetDouble("1.4", 1.79). SetString("k1.5", "4"). SetBytes("k1.6", []byte{65, 66}). - SetDate("k1.7", time.Now()) + SetDate("k1.7", gotime.Now()) // an array root.SetNewArray("k2"). @@ -132,7 +132,7 @@ func TestConverter(t *testing.T) { AddDouble(3.0). AddString("4"). AddBytes([]byte{65}). - AddDate(time.Now()) + AddDate(gotime.Now()) // plain text root.SetNewText("k3"). @@ -154,7 +154,7 @@ func TestConverter(t *testing.T) { pbPack := converter.ToChangePack(d1.CreateChangePack()) pack, err := converter.FromChangePack(pbPack) - pack.MinSyncedTicket = time2.MaxTicket + pack.MinSyncedTicket = time.MaxTicket assert.NoError(t, err) d2 := document.New("c1", "d1") diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 6a2313156..0b038c921 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -22,13 +22,12 @@ import ( "runtime" "testing" - "github.com/yorkie-team/yorkie/pkg/document/json" - "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/api/converter" "github.com/yorkie-team/yorkie/pkg/document" "github.com/yorkie-team/yorkie/pkg/document/checkpoint" + "github.com/yorkie-team/yorkie/pkg/document/json" "github.com/yorkie-team/yorkie/pkg/document/proxy" "github.com/yorkie-team/yorkie/pkg/document/time" "github.com/yorkie-team/yorkie/testhelper" From 1eefbb3c59da096a1f7496b71df2cc9189088b82 Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 20 Dec 2020 18:39:45 +0900 Subject: [PATCH 12/14] Remove removedNodeMapKey type We can use the ID createdAt and offset in Node.id to get the unique value that can identify the node. --- pkg/document/json/rga_tree_split.go | 33 ++++++++++++----------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/pkg/document/json/rga_tree_split.go b/pkg/document/json/rga_tree_split.go index cf7c5e367..1c8a8e45f 100644 --- a/pkg/document/json/rga_tree_split.go +++ b/pkg/document/json/rga_tree_split.go @@ -76,6 +76,10 @@ func (t *RGATreeSplitNodeID) hasSameCreatedAt(id *RGATreeSplitNodeID) bool { return t.createdAt.Compare(id.createdAt) == 0 } +func (t *RGATreeSplitNodeID) key() string { + return fmt.Sprintf("%s:%d", t.createdAt.Key(), t.offset) +} + type RGATreeSplitNodePos struct { id *RGATreeSplitNodeID relativeOffset int @@ -233,22 +237,14 @@ func (t *RGATreeSplitNode) Value() RGATreeSplitValue { return t.value } -// removedNodeMapKey is a key of removedNodeMap. -// The shape of the key is '{createdAt.Key()}:{offset}'. -type removedNodeMapKey string - -// createRemovedNodeMapKey generates keys for removedNodes. -func createRemovedNodeMapKey(createdAtKey string, offset int) removedNodeMapKey { - key := fmt.Sprintf("%s:%d", createdAtKey, offset) - return removedNodeMapKey(key) -} - type RGATreeSplit struct { initialHead *RGATreeSplitNode treeByIndex *splay.Tree treeByID *llrb.Tree - removedNodeMap map[removedNodeMapKey]*RGATreeSplitNode + // removedNodeMap is a map that holds tombstone nodes + // when the edit operation is executed. + removedNodeMap map[string]*RGATreeSplitNode } func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { @@ -260,7 +256,7 @@ func NewRGATreeSplit(initialHead *RGATreeSplitNode) *RGATreeSplit { initialHead: initialHead, treeByIndex: treeByIndex, treeByID: treeByID, - removedNodeMap: make(map[removedNodeMapKey]*RGATreeSplitNode), + removedNodeMap: make(map[string]*RGATreeSplitNode), } } @@ -397,7 +393,7 @@ func (s *RGATreeSplit) edit( // 02. delete between from and to nodesToDelete := s.findBetween(fromRight, toRight) - latestCreatedAtMap, removedNodeMapByCreatedAt := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) + latestCreatedAtMap, removedNodeMapByNodeKey := s.deleteNodes(nodesToDelete, latestCreatedAtMapByActor, editedAt) var caretID *RGATreeSplitNodeID if toRight == nil { @@ -414,7 +410,7 @@ func (s *RGATreeSplit) edit( } // 04. add removed node - for key, removedNode := range removedNodeMapByCreatedAt { + for key, removedNode := range removedNodeMapByNodeKey { s.removedNodeMap[key] = removedNode } @@ -435,9 +431,9 @@ func (s *RGATreeSplit) deleteNodes( candidates []*RGATreeSplitNode, latestCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, -) (map[string]*time.Ticket, map[removedNodeMapKey]*RGATreeSplitNode) { +) (map[string]*time.Ticket, map[string]*RGATreeSplitNode) { createdAtMapByActor := make(map[string]*time.Ticket) - removedNodeMap := make(map[removedNodeMapKey]*RGATreeSplitNode) + removedNodeMap := make(map[string]*RGATreeSplitNode) for _, node := range candidates { actorIDHex := node.createdAt().ActorIDHex() @@ -462,8 +458,7 @@ func (s *RGATreeSplit) deleteNodes( createdAtMapByActor[actorIDHex] = createdAt } - key := createRemovedNodeMapKey(createdAt.Key(), node.id.offset) - removedNodeMap[key] = node + removedNodeMap[node.id.key()] = node } } @@ -539,7 +534,7 @@ func (s *RGATreeSplit) cleanupRemovedNodes(ticket *time.Ticket) int { s.treeByIndex.Delete(node.indexNode) s.purge(node) s.treeByID.Remove(node.id) - delete(s.removedNodeMap, removedNodeMapKey(node.createdAt().Key())) + delete(s.removedNodeMap, node.id.key()) count++ } } From 1cc149a6371da46392746ad516024d3729b0344c Mon Sep 17 00:00:00 2001 From: dc7303 Date: Sun, 20 Dec 2020 21:48:06 +0900 Subject: [PATCH 13/14] Clean up code with lint error Fixed an error notified by Lint after master merge. --- pkg/document/change/context.go | 1 + pkg/document/document_test.go | 4 +++- pkg/document/json/element.go | 2 +- testhelper/testhelper.go | 1 + 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/document/change/context.go b/pkg/document/change/context.go index 82dd319c5..b33b3bfea 100644 --- a/pkg/document/change/context.go +++ b/pkg/document/change/context.go @@ -78,6 +78,7 @@ func (c *Context) RegisterRemovedElementPair(parent json.Container, deleted json c.root.RegisterRemovedElementPair(parent, deleted) } +// RegisterRemovedNodeTextElement registers a given text type element to hash table. func (c *Context) RegisterRemovedNodeTextElement(textType json.TextElement) { c.root.RegisterRemovedNodeTextElement(textType) } diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 46b0bec55..b1ab67596 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -227,7 +227,8 @@ func TestDocument(t *testing.T) { assert.Equal(t, expected, doc.Marshal()) assert.Equal(t, 1, doc.GarbageLen()) - expected = `{"k1":[{"attrs":{"b":"1"},"val":"Hi"},{"attrs":{"b":"1"},"val":" "},{"attrs":{},"val":"j"},{"attrs":{"b":"1"},"val":"ane"}]}` + expected = `{"k1":[{"attrs":{"b":"1"},"val":"Hi"},{"attrs":{"b":"1"},"val":" "},` + + `{"attrs":{},"val":"j"},{"attrs":{"b":"1"},"val":"ane"}]}` err = doc.Update(func(root *proxy.ObjectProxy) error { text := root.GetRichText("k1") text.Edit(0, 5, "Hi", map[string]string{"b": "1"}) @@ -300,6 +301,7 @@ func TestDocument(t *testing.T) { return nil }, "initial") fmt.Println("-----long text by one node") + assert.NoError(t, err) printMemStats(doc.RootObject()) // 05. Modify one node multiple times diff --git a/pkg/document/json/element.go b/pkg/document/json/element.go index 9402062c2..0faf323da 100644 --- a/pkg/document/json/element.go +++ b/pkg/document/json/element.go @@ -32,7 +32,7 @@ type Container interface { DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) Element } -// TextType represents Text or RichText. +// TextElement represents Text or RichText. type TextElement interface { Element removedNodesLen() int diff --git a/testhelper/testhelper.go b/testhelper/testhelper.go index 35aaa62eb..e6d2b7a12 100644 --- a/testhelper/testhelper.go +++ b/testhelper/testhelper.go @@ -56,6 +56,7 @@ func TestDBName() string { return fmt.Sprintf("test-%s-%d", yorkie.DefaultMongoYorkieDatabase, testStartedAt) } +// TestRoot returns the root func TestRoot() *json.Root { return json.NewRoot(json.NewObject(json.NewRHTPriorityQueueMap(), time.InitialTicket)) } From 151a901bfac1864608b20ec01aef254af50d2c8e Mon Sep 17 00:00:00 2001 From: Dongcheol Choe <40932237+dc7303@users.noreply.github.com> Date: Mon, 21 Dec 2020 09:32:32 +0900 Subject: [PATCH 14/14] Cleanup root_test.go Sort the import groups. Co-authored-by: Youngteac Hong --- pkg/document/json/root_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/document/json/root_test.go b/pkg/document/json/root_test.go index c00c158e2..a1549a3e3 100644 --- a/pkg/document/json/root_test.go +++ b/pkg/document/json/root_test.go @@ -19,11 +19,10 @@ package json_test import ( "testing" - "github.com/yorkie-team/yorkie/pkg/document/time" - "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/pkg/document/json" + "github.com/yorkie-team/yorkie/pkg/document/time" "github.com/yorkie-team/yorkie/testhelper" )