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

Casper Norrbin cnorrbin at openjdk.org
Mon Dec 2 15:46:17 UTC 2024


On Mon, 2 Dec 2024 09:19:55 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi everyone,
>> 
>> This effort began as an exploration of replacing the current NMT treap with a red-black tree. Along the way, I discovered that others were also interested in having a general-purpose tree structure available within HotSpot.
>> 
>> The red-black tree is designed to serve as a drop-in replacement for the existing NMT treap, keeping a nearly identical interface. However, I’ve also added a few additional requested features, such as an iterator.
>> 
>> Testing builds off the treap tests, adding a few extra that inserts/removes and checks that the tree is correct. Testing uses the function `verify_self`, which iterates over the tree and checks att all red-black tree properties hold. Additionally, the tree has been tested in vmatree instead of the treap without any errors.
>> 
>> For those who may want to revisit the fundamentals of red-black trees, [Wikipedia](https://en.wikipedia.org/wiki/Red%E2%80%93black_tree) offers a great summary with tables covering the various balancing cases. Alternatively, your favorite data structure book could provide even more insight.
>
> src/hotspot/share/utilities/rbTree.hpp line 377:
> 
>> 375:   }
>> 376: 
>> 377:   void remove_inner(RBNode* node) {
> 
> Seems like this function is made to remove nodes whose color is black and whose removal would cause an imbalance. Maybe we can have a better name for this function or documentation, and also a few asserts?

Renamed to `remove_black_leaf()`

> src/hotspot/share/utilities/rbTree.hpp line 381:
> 
>> 379:     RBNode* parent = node->_parent;
>> 380:     while (parent != nullptr) {
>> 381:       RBNode* sibling = node->is_left_child() ? parent->_right : parent->_left;
> 
> I'm missing why this is guaranteed to not be null

Once we reach here, node is a leaf node and black, which means that a sibling has to exist to not cause a black-violation where the path through the sibling has one less black node.
I added an explaining comment + changed the function name to be clearer.

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

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


More information about the hotspot-dev mailing list