RFR: 8356455: ZGC: Replace ZIntrusiveRBTree with IntrusiveRBTree [v2]

Stefan Karlsson stefank at openjdk.org
Tue May 13 04:31:56 UTC 2025


On Thu, 8 May 2025 13:01:07 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> [JDK-8350441](https://bugs.openjdk.org/browse/JDK-8350441) was implemented before IntrusiveRBTree was introduced, and as such implemented its own intrusive red-black tree. Now that a shared data structure implementation is available, use that instead.
>> 
>> The switch is straight forward, and the O(1) left and right most node lookup which ZIntrusiveRBTree implements that IntrusiveRBTree does not is trivial to implement on top of the tree.
>> 
>> Initial performance evaluation shows no difference between the two implementations. And the functional testing passes.
>> 
>> Tested Oracle Supported platforms, Oracle tier1-8 ZGC testing tasks.
>
> Axel Boldt-Christmas has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Use private inheritance
>  - Separate tree logic to own class

Thanks for doing this cleanup. I have a few nits below.

src/hotspot/share/gc/z/zMappedCache.cpp line 308:

> 306: 
> 307:   // Replace in size-class lists
> 308:   _tree.replace(old_node, new_node, cursor);

This code was changed from:

  // Replace in tree
  _tree.replace(entry->node_addr(), cursor);

  // Replace in size-class lists


to:

  // Replace in size-class lists
  _tree.replace(old_node, new_node, cursor);


It seems like something went wrong with the comments.

src/hotspot/share/gc/z/zMappedCache.cpp line 672:

> 670:   // use is_empty_error_reporter_safe and size_error_reporter_safe on the size
> 671:   // class lists.
> 672:   const size_t entry_count = _tree.size();

There doesn't seem to be an `Atomic::load` or `volatile` to make sure that we honor the comment about reading only once.

src/hotspot/share/gc/z/zMappedCache.hpp line 32:

> 30: #include "gc/z/zList.hpp"
> 31: #include "utilities/globalDefinitions.hpp"
> 32: #include "utilities/rbTree.hpp"

Sort order.

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

PR Review: https://git.openjdk.org/jdk/pull/25112#pullrequestreview-2835230617
PR Review Comment: https://git.openjdk.org/jdk/pull/25112#discussion_r2085893256
PR Review Comment: https://git.openjdk.org/jdk/pull/25112#discussion_r2085896187
PR Review Comment: https://git.openjdk.org/jdk/pull/25112#discussion_r2085896390


More information about the hotspot-gc-dev mailing list