RFR: 8345314: Add a red–black tree as a utility data structure [v12]

[Johan Sjölen]

On Thu, 16 Jan 2025 07:03:55 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Casper Norrbin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   clarified comment
>
> src/hotspot/share/utilities/rbTree.hpp line 167:
> 
>> 165:   // Removes the given node from the tree.
>> 166:   // Returns true if the node was successfully removed, false otherwise.
>> 167:   bool remove(RBNode* node);
> 
> I find it confusing that removing a node can fail. It seems to be to handle `nullptr`. I'd rather restrict this and have `bool remove(const K& k)` check the return value of `find_node`.

Wouldn't that require two traversals down the tree? Would a `enum RemovalResult { Removed, NotFound }` be preferred?

> test/hotspot/gtest/utilities/test_rbtree.cpp line 111:
> 
>> 109:     };
>> 110: 
>> 111:     constexpr const int up_to = 10;
> 
> Suggestion:
> 
>     constexpr int up_to = 10;

TIL, I usually put both.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22360#discussion_r1918332766
PR Review Comment: https://git.openjdk.org/jdk/pull/22360#discussion_r1918333661