tree: Refactor RangeMap to use a B-tree #23497
Conversation
github-actions
bot
added
base: main
| * Retrieves all entries from the rangeMap. | ||
| * @returns An array of RangeEntryResult objects, each containing the start index, length, and value of a contiguous range. |
There was a problem hiding this comment.
| * Retrieves all entries from the rangeMap. | |
| * @returns An array of RangeEntryResult objects, each containing the start index, length, and value of a contiguous range. | |
| * Retrieves all entries from the rangeMap. |
There was a problem hiding this comment.
I didn't think that the @return comment had a lot of value: it't not saying anything that the signature doesn't convey.
| @@ -147,73 +105,66 @@ export class RangeMap<T> { | |||
| * @param length - The length of the range to delete. | |||
| */ | |||
| public delete(start: number, length: number): void { | |||
There was a problem hiding this comment.
Looking at the code in get and delete, I wonder if there's some intermediate interface that is begging to be born.
This might be something like getInternalEntries(start: number, length: number): Iterable<[number, RangeEntry<T>]> where the first element of the iterator may represent a range that starts before start and the last element of the iterator may represent a range that ends after start + length.
Such a function would in part be based on the BTree's getRange function.
The implementation for get would only read the first entry from the iterable and return a potentially truncated version of it.
The implementation for delete would read all entries, splitting the first and last if needed. (Splitting might be another internal helper that makes sense)
I'm happy to chat about this. I don't take it for granted that the motivation for this is high especially now that you've written this code.
I do wonder whether it would then enable us to expose a variant of get that returns all relevant entries for a range (with an implementation that is faster than repeatedly calling the current get).
There was a problem hiding this comment.
I agree about making a variant that returns all entries and was planning to do that soon. I'll try refactoring the implementation as suggested.
Description
Reimplemented RangeMap to store entries in a B-tree instead of an array to improve asymptotic performance.