From dd6020083c1beb75a350c68be7f503f16ae090f2 Mon Sep 17 00:00:00 2001 From: keep94 Date: Tue, 4 Oct 2016 10:02:35 -0700 Subject: [PATCH 1/2] Fix memory leak in freeNode and splitNode. --- btree.go | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/btree.go b/btree.go index 09dcb32..b31c194 100644 --- a/btree.go +++ b/btree.go @@ -87,7 +87,9 @@ func (f *FreeList) newNode() (n *node) { if index < 0 { return new(node) } - f.freelist, n = f.freelist[:index], f.freelist[index] + n = f.freelist[index] + f.freelist[index] = nil + f.freelist = f.freelist[:index] return } @@ -153,6 +155,16 @@ func (s *items) pop() (out Item) { return } +// truncate truncates this instance at index so that it contains only the +// first index items. index must be less than or equal to length. +func (s *items) truncate(index int) { + l := len(*s) + for i := index; i < l; i++ { + (*s)[i] = nil + } + *s = (*s)[:index] +} + // find returns the index where the given item should be inserted into this // list. 'found' is true if the item already exists in the list at the given // index. @@ -198,6 +210,16 @@ func (s *children) pop() (out *node) { return } +// truncate truncates this instance at index so that it contains only the +// first index children. index must be less than or equal to length. +func (s *children) truncate(index int) { + l := len(*s) + for i := index; i < l; i++ { + (*s)[i] = nil + } + *s = (*s)[:index] +} + // node is an internal node in a tree. // // It must at all times maintain the invariant that either @@ -216,10 +238,10 @@ func (n *node) split(i int) (Item, *node) { item := n.items[i] next := n.t.newNode() next.items = append(next.items, n.items[i+1:]...) - n.items = n.items[:i] + n.items.truncate(i) if len(n.children) > 0 { next.children = append(next.children, n.children[i+1:]...) - n.children = n.children[:i+1] + n.children.truncate(i + 1) } return item, next } @@ -532,14 +554,9 @@ func (t *BTree) newNode() (n *node) { } func (t *BTree) freeNode(n *node) { - for i := range n.items { - n.items[i] = nil // clear to allow GC - } - n.items = n.items[:0] - for i := range n.children { - n.children[i] = nil // clear to allow GC - } - n.children = n.children[:0] + // clear to allow GC + n.items.truncate(0) + n.children.truncate(0) n.t = nil // clear to allow GC t.freelist.freeNode(n) } From 311d74f9ecd35bf58ed11142bb01f0cbb98c3990 Mon Sep 17 00:00:00 2001 From: keep94 Date: Wed, 5 Oct 2016 11:18:25 -0700 Subject: [PATCH 2/2] gconnell comments. --- btree.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/btree.go b/btree.go index b31c194..6088670 100644 --- a/btree.go +++ b/btree.go @@ -68,6 +68,11 @@ const ( DefaultFreeListSize = 32 ) +var ( + nilItems = make(items, 16) + nilChildren = make(children, 16) +) + // FreeList represents a free list of btree nodes. By default each // BTree has its own FreeList, but multiple BTrees can share the same // FreeList. @@ -158,11 +163,11 @@ func (s *items) pop() (out Item) { // truncate truncates this instance at index so that it contains only the // first index items. index must be less than or equal to length. func (s *items) truncate(index int) { - l := len(*s) - for i := index; i < l; i++ { - (*s)[i] = nil + var toClear items + *s, toClear = (*s)[:index], (*s)[index:] + for len(toClear) > 0 { + toClear = toClear[copy(toClear, nilItems):] } - *s = (*s)[:index] } // find returns the index where the given item should be inserted into this @@ -213,11 +218,11 @@ func (s *children) pop() (out *node) { // truncate truncates this instance at index so that it contains only the // first index children. index must be less than or equal to length. func (s *children) truncate(index int) { - l := len(*s) - for i := index; i < l; i++ { - (*s)[i] = nil + var toClear children + *s, toClear = (*s)[:index], (*s)[index:] + for len(toClear) > 0 { + toClear = toClear[copy(toClear, nilChildren):] } - *s = (*s)[:index] } // node is an internal node in a tree.