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

Casper Norrbin cnorrbin at openjdk.org
Fri Jan 17 16:32:44 UTC 2025


On Thu, 16 Jan 2025 12:02:31 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> 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?

`remove(RBNode* node)` doesn't traverse the tree, so it is only ever one traversal. I moved the check to `remove(const K& k)` like you suggested, with an added assumption with a comment and assert that the node has to be valid.

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

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


More information about the hotspot-dev mailing list