RFR: 8342730: Get rid of SummaryDiff in VMATree
Paul Hübner
phubner at openjdk.org
Wed Aug 27 13:04:46 UTC 2025
On Wed, 13 Aug 2025 13:33:36 GMT, Afshin Zafari <azafari at openjdk.org> wrote:
> `SummaryDiff` result of any `register_mapping()` call, is passed into the function rather than returned. This enables avoiding copying the structure to/from the stack.
>
> Tests:
> local linux-x64-debug: NMT gtests and JTREGs passed.
Thanks for the work, Afshin. I just have one small question and a few nitpicks, but other than that it looks in good shape to be merged.
src/hotspot/share/nmt/vmatree.cpp line 248:
> 246: const RegionData& metadata, VMATree::SummaryDiff& diff, bool use_tag_inplace) {
> 247:
> 248: diff.clear();
I'm not sure I follow why this is introduced. Is there a situation where we/it makes sense to re-use `SumaryDiff`s?
test/hotspot/gtest/nmt/test_regions_tree.cpp line 102:
> 100: NativeCallStack ncs;
> 101: VMATree::RegionData rd = rt.make_region_data(ncs, mtTest);
> 102: VMATree::SummaryDiff diff;
Nit: it's unused in the actual test/assertions right? Would call it `not_used` to be consistent with above test code then.
test/hotspot/gtest/nmt/test_regions_tree.cpp line 121:
> 119: NativeCallStack ncs;
> 120: VMATree::RegionData rd = rt.make_region_data(ncs, mtTest);
> 121: VMATree::SummaryDiff diff;
Same naming nit here.
test/hotspot/gtest/nmt/test_regions_tree.cpp line 137:
> 135: NativeCallStack ncs;
> 136: VMATree::RegionData rd = rt.make_region_data(ncs, mtTest);
> 137: VMATree::SummaryDiff diff;
Same naming nit here.
test/hotspot/gtest/nmt/test_vmatree.cpp line 1112:
> 1110: {// Check committing into a reserved region inherits the call stacks
> 1111: Tree tree;
> 1112: VMATree::SummaryDiff diff;
Same nitpick here as in the other places in this file, I think it's more clear if we used `not_used` for cases where we discard the result. I'll stop pointing out every place this happens, consider this comment for the whole file.
-------------
PR Review: https://git.openjdk.org/jdk/pull/26761#pullrequestreview-3159649505
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2303829869
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2303845475
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2303847970
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2303848336
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2303861190
More information about the hotspot-runtime-dev
mailing list