[Lldb-commits] [PATCH] D74759: Treat RangeDataVector as an augmented BST
Unnar Freyr Erlendsson via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Feb 26 05:06:12 PST 2020
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>
More information about the lldb-commits
mailing list