Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

Performance improvements to iteration#1

Closed
cstockton wants to merge 4 commits intogoogle:masterfrom
cstockton:master
Closed

Performance improvements to iteration#1
cstockton wants to merge 4 commits intogoogle:masterfrom
cstockton:master

Conversation

@cstockton
Copy link
Copy Markdown

Created a few more iterators to be used by the public interface for performance improvements. I did this in two commits, the second one changes the private interface signatures from funcs (technically same signature as ItemIterator) to Item, but since the public interface never actually exposes funcs for ascending I figured this would be okay.

Before:

~/.../github.com/cstockton/gbtree$ go test -test.bench=.*
1
PASS
BenchmarkInsert  3000000           411 ns/op
BenchmarkDelete  3000000           424 ns/op
BenchmarkGet     5000000           363 ns/op
BenchmarkIterateAscend      3000        474049 ns/op
BenchmarkIterateAscendLessThan      5000        274409 ns/op
BenchmarkIterateAscendGreaterOrEqual        5000        277208 ns/op
BenchmarkIterateAscendRange    10000        216873 ns/op
ok      github.com/cstockton/gbtree 16.429s

After:

~/.../github.com/cstockton/btree$ go test -test.bench=.*
1
PASS
BenchmarkInsert  3000000           408 ns/op
BenchmarkDelete  3000000           422 ns/op
BenchmarkGet     5000000           360 ns/op
BenchmarkIterateAscend      3000        431754 ns/op
BenchmarkIterateAscendLessThan      5000        257723 ns/op
BenchmarkIterateAscendGreaterOrEqual        5000        257560 ns/op
BenchmarkIterateAscendRange    10000        205764 ns/op
ok      github.com/cstockton/btree  15.984s

btree.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use n.items.find here to speed this up a bit? Now that you're looking for an item, no reason we can't binary-search for it.

@gconnell
Copy link
Copy Markdown
Contributor

When you get a chance, could you sign the Google CLA? It's available at https://cla.developers.google.com/about/google-individual, and it needs to be signed before I accept code from you. Basically, it says "you still own the code, but you allow us to use it".

@cstockton
Copy link
Copy Markdown
Author

I signed it and associated it to this github account.

@gconnell
Copy link
Copy Markdown
Contributor

Thanks for signing! I'll wait for your binary search changes before merging.

@cstockton
Copy link
Copy Markdown
Author

Hello, sorry was traveling over the last week but I just made the change you requested. Yielded a fair gain which of course favors larger degrees.

BenchmarkIterateAscendGreaterOrEqual       10000        230029 ns/op
BenchmarkIterateAscendRange    10000        183623 ns/op

That said, I see a few other areas for improvement that would be pretty simple to do but would be larger changes. For example I saw that every single node contains a reference to BTree which is only used to freeNode and newNode. Simply moving them to package level funcs allows the reference to the tree to be removed.. which yields some more gains:

BenchmarkInsert  5000000           394 ns/op
BenchmarkDelete  3000000           409 ns/op
BenchmarkGet     5000000           355 ns/op
BenchmarkIterateAscend      3000        430221 ns/op
BenchmarkIterateAscendLessThan      5000        255032 ns/op
BenchmarkIterateAscendGreaterOrEqual       10000        231429 ns/op
BenchmarkIterateAscendRange    10000        182023 ns/op
ok      github.com/cstockton/btree-find 17.269s

With the only use of t* removed, we could take out the reference to BTree entirely- which opens us up to allowing a new pointer to be introduced into each node without incurring any net-loss.. It might be nice to have each node reference the parent node to allow a morris traversal which I believe would be much faster. Just some food for thought.

EDIT:
Should I close this out?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants