Fix memory leak in freeNode and splitNode. #11
Conversation
| // 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) |
There was a problem hiding this comment.
Same comment as above, re: copy() and loop unrolling.
There was a problem hiding this comment.
Done with similar modifications as above.
| @@ -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) { | |||
There was a problem hiding this comment.
What do you think of something like this, to unroll the loop a bit?
// at global scope
var nullItems = make(items, 16)
func (s *items) truncate(index int) {
*s, toClear := (*s)[:index], (*s)[index:]
for len(toClear) > 0 {
toClear = toClear[copy(toClear, nullItems):]
}
}There was a problem hiding this comment.
Done. Except I had to add "var toClear items" and change := to = to make compiler happy.
Very clever code. Thank you.
There was a problem hiding this comment.
I also changed "nullItems" to "nilItems" since "null" reminds me of JAVA :-)
|
I see no significant change in Insert, Delete, or Get. benchmarkAscend seems to run slightly faster with my PR (230us vs 250us). I see no significant change with the other Ascend/Descend benchmarks. I don't understand why benchmarkAscend would run faster with my PR as ascending the btree is a read-only operation and shouldn't be splitting nodes or touching the freelist. |
|
Thanks for the comments. Sorry I didn't see them before. Very clever code you proposed. I integrated your changes and reran the benchmarks. I saw no significant change in the performance of insert and delete. |
|
BenchmarkAscend still runs faster with this PR (220us vs 250us) I don't understand why. |
google/btree#11 Release note: None
google/btree#11 Release note: None
google/btree#11 Release note: None
google/btree#11 Release note: None
This PR includes two important changes to help the GC reclaim memory. Both changes involve making sure that elements past the end of a slice but still within the slice's capacity can be gced.
The first change is freelist.newNode() setting the last element to nil in the freelist before shrinking the size of the freelist by 1.
The second change is adding a truncate method to the items and children types and making sure that callers use truncate instead of something like items := items[:i]. truncate() sets all the items past the end of the slice to nil allowing them to be GCed.