Add PointIndex and ClosestPointQuery - #274
Conversation
Points are stored in a plain sorted slice keyed by leaf CellID. Add and Remove maintain sort order by shifting elements, so each mutation is O(n). This implementation is not a btree and is therefore not suitable for large datasets or frequently mutated indexes; it is designed for the build-once, query-many pattern.
It uses
That's archived. https://github.com/google/btree Can you do some research on other b-tree options? |
With a quick search, there don’t seem to be many B-tree implementations in Go. I only found two GitHub repositories from the same author, Josh Baker (Tidwall):
The tidwall/btree project is a fairly complete Go B-tree implementation with support for generics, ordered maps/sets, iterators, bulk loading, and copy-on-write features. |
This seems fine.
I would give this more burn-in time. |
jmr
left a comment
jmr
left a comment
There was a problem hiding this comment.
Probably some more comments later.
| @@ -0,0 +1,496 @@ | |||
| // Copyright 2015 Google Inc. All rights reserved. | |||
There was a problem hiding this comment.
Copyright 2025-2026 Google LLC. All rights reserved.
| // this after copying an iterator (it2 = *it) and before calling Next() or | ||
| // Prev() across a CellID boundary, to decouple the copy's cursor from the | ||
| // original. | ||
| func (it *PointIndexIterator[Data]) Refresh() { |
There was a problem hiding this comment.
Refresh seems a bit weird. What if we just don't support iterator copying?
Are you implementing this because you want to use it or because you want to implement something?
If the first, will no copy still work for you?
There's also BTreeG which supports duplicate keys. That could work.
| query.opts.distanceLimit = s1.ChordAngleFromAngle(s1.Angle(frac) * queryRadius) | ||
| } | ||
| if randomUniformInt(2) != 0 { | ||
| maxErrFrac := 1e-4 + math.Exp(randomUniformFloat64(0, 1)*math.Log(1.0)) |
| func (it *PointIndexIterator[Data]) LocatePoint(target Point) bool { | ||
| id := cellIDFromPoint(target) | ||
| it.Seek(id) | ||
| if !it.Done() && it.CellID().RangeMin() <= id { |
There was a problem hiding this comment.
We only have leaf cells, so this can be simplifed. Comment why it's different than C++, though.
| return true | ||
| } | ||
| // Already at the first entry; restore the cursor so Next() still works. | ||
| it.iter.Seek(it.currentID) |
There was a problem hiding this comment.
Do we need this? Check what btree iterator does and add a comment.
jmr
changed the title
2 participants
Summary
Implementation note / question
The current implementation stores entries in a plain sorted slice. Add and Remove maintain sort order by shifting elements, making each mutation O(n). This matches the build-once, query-many usage pattern described in the doc comment,
but is not suitable for large datasets or frequently mutated indexes.
The C++ S2 library uses std::map (a red-black tree) for this type, which gives O(log n) mutations. Would it make sense to introduce github.com/google/btree as a dependency to back this index with a proper tree structure? That would
bring mutations down to O(log n) at the cost of adding a new module dependency.
Happy to rework the implementation if the project is open to that dependency, or to keep the current approach and document the limitation more prominently if not.