[Lldb-commits] [PATCH] D74759: Treat RangeDataVector as an augmented BST

[Unnar Freyr Erlendsson via Phabricator via lldb-commits]

unnar updated this revision to Diff 246679.
unnar marked 5 inline comments as done.
unnar added a comment.

In D74759#1880499 <https://reviews.llvm.org/D74759#1880499>, @labath wrote:

> Have you done a proper complexity analysis here? I doubt the `O(log n)` claim is true in general. It would have to be at least `O(m + log n)` (m - number of elements found), but it's not clear to me whether even this is true in general. (However, I believe this can achieve ~~log(n) for non-degenerate cases.)


I should have been more specific, searching for any interval that contains a point can be done in `O(log n)` but in our case where we are searching for all intervals that contain the point and as Jarin said we believe it takes `O(m log n)` (I admit I have not done a thorough analysis of the time complexity myself). The theoretical best you can do is indeed `O(m + log n)`.

> The implementation itself needs some work though. My incomplete list of comments is:
> 
> - replace `int` with `size_t` and closed intervals with half-open ones

Done.

> - let's move the computation of the upper bound into the "Sort" function. sorting is O(n log(n)), this is O(n) -- we can just hide it there.

That was my first thought but I decided to have it generated lazily instead, I will move it back to the sort.

> - make private functions private

Done.

> - we should avoid the business of figuring out what is the suitable "minimum" value of B by ensuring we call the recursive function on non-empty intervals

Done.

> - clang-format the patch

Done.

> For testing you should add some c++ unit tests for the relevant interfaces.

Done. Do you think we should also unit test ComputeUpperBounds or just the interface?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74759/new/

https://reviews.llvm.org/D74759

Files:
  lldb/include/lldb/Utility/RangeMap.h
  lldb/unittests/Utility/RangeMapTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74759.246679.patch
Type: text/x-patch
Size: 6460 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200226/24ee9310/attachment-0001.bin>