-
Notifications
You must be signed in to change notification settings - Fork 2
Reverse iterators #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Reverse iterators #138
Conversation
Add support for iterating over the tree or prefix search in reverse. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Add the AllReverse and PrefixReverse methods to iterate over the results in reverse order. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Add [AllReverse], [AllReverseWatch], [ListReverse], [ListReverseWatch], [PrefixReverse] and [PrefixReverseWatch] queries to the [Table] interface. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Benchmark results: BenchmarkDB_FullIteration_All-8 978 1189033 ns/op 84101978 objects/sec 104 B/op 4 allocs/op BenchmarkDB_FullIteration_AllReverse-8 650 1848977 ns/op 54083963 objects/sec 136 B/op 5 allocs/op BenchmarkDB_FullIteration_Prefix-8 1018 1182365 ns/op 84576225 objects/sec 136 B/op 5 allocs/op BenchmarkDB_FullIteration_PrefixReverse-8 738 1637465 ns/op 61070025 objects/sec 136 B/op 5 allocs/op Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
|
|
Before I read the code - can you outline why this is needed? |
There isn't an immediate use case, but it's something I've been wanting to add from the beginning. It's also a nicely incremental change that doesn't touch existing code paths that much. So the strongest rational right now is just that I've always wanted this to be part of the API. Some of the useful things with this that I can imagine are e.g. iterating over the revision index in newly changed first order (to for example reconcile new objects first) or iterating over some sort of priority/cost/etc. index in reverse order. Oh and just realized that for debugging we might want the EDIT: Oh wait, we can't really iterate the revision index in reverse order nicely currently. Can't give the index name to |
bimmlerd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaving review here at first commit - API wants to marinate a bit according to Jussi
| // Iterator returns an iterator for all objects. | ||
| Iterator() Iterator[T] | ||
|
|
||
| // ReverseIterator returns an iterator for all objects in reverse order. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "reverse order" suggests that Iterator has a defined order. Is that the case and should it be documented?
| tree := New[int]() | ||
| it := tree.ReverseIterator() | ||
| for range it.All { | ||
| t.Fatalf("All yielded value from empty tree") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same test for Next?
| for j := uint64(1); j <= numObjectsToInsert; j++ { | ||
| _, _, tree = tree.Insert(uint64Key(j), j) | ||
| } | ||
| b.ResetTimer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: reset timer etc ;)
|
Drafted until I've thought through how to change the API to allow reverse iterating over the revision index nicely... |
Add reverse-iteration support across StateDB: new Table APIs (AllReverse, ListReverse, PrefixReverse + watch variants), index-level reverse plumbing, and efficient reverse iterators for part and lpm.
Example usage:
Benchmarks: