RFR: 8345314: Add a red–black tree as a utility data structure [v7]
Casper Norrbin
cnorrbin at openjdk.org
Thu Jan 9 13:14:39 UTC 2025
On Tue, 7 Jan 2025 18:38:12 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
>> Casper Norrbin has updated the pull request incrementally with one additional commit since the last revision:
>>
>> renamed coloring functions
>
> src/hotspot/share/utilities/rbTree.inline.hpp line 438:
>
>> 436:
>> 437: node = curr;
>> 438: }
>
> @tstuefe,
>
> Seems like nodes are not stable. To quote Wikipedia:
>
>>- When the deleted node has 2 children (non-NIL), then we can swap its value with its in-order successor (the leftmost child of the right subtree), and then delete the successor instead. Since the successor is leftmost, it can only have a right child (non-NIL) or no child at all.
>
> This is quite unfortunate, as it becomes very difficult (impossible?) to have intrusive RB-trees with this.
>
> @caspernorrbin,
>
> Finding information on this in the literature seems like it takes quite a bit of digging. Can't we replace this with swapping everything *but* the key and value?
>
> ```c++
> if (node->_left != nullptr && node->_right != nullptr) { // node has two children
> RBNode* curr = node->_right;
> while (curr->_left != nullptr) {
> curr = curr->_left;
> }
> // Swap parent, left, right, and color
> std::swap(curr->_parent, node->_parent);
> std::swap(curr->_left, node->_left);
> std::swap(curr->_right, node->_right);
> std::swap(curr->_color, node->_color);
> // Swap the parent's child pointer
> if (curr->_parent->_left == node) curr->_parent->_left = curr;
> if (curr->_parent->_right == node) curr->_parent->_right = curr;
> // Set the children's parent pointers
> curr->_left->_parent = curr; curr->_right->_parent = curr;
>
> // and then the identical writes for node
> if (node->_parent->_left == curr) node->_parent->_left = node;
> if (node->_parent->_right == curr) node->_parent->_right = node;
> // Set the children's parent pointers
> node->_left->_parent = node; node->_right->_parent = node;
>
> // Done, they've swapped places in the tree.
> // node = curr; <- Don't do this anymore, should still delete node
> }
Nodes are now stable! This was not something I had considered, but it's possible with what Johan suggested above. Previously, keys and values were swapped around between nodes during deletion but now we instead re-point the node's pointers instead. With this the key and value stays constant in a node through its lifetime, given the user does not themself change the value.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22360#discussion_r1908774771
More information about the hotspot-dev
mailing list