RFR: 8342730: Get rid of SummaryDiff in VMATree [v2]

[Afshin Zafari]

On Wed, 27 Aug 2025 12:47:03 GMT, Paul Hübner <phubner at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   better naming for diff
>
> 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?

It's a bit backward support. The `diff` was constructed locally inside this method and thus was clean. All the callers of this method (mostly from tests) also assume so. Therefore, without this new explicit `clean()` all of them should have called the `clean()` themselves.

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

Good point. 
Done.

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

Done.

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

Done.

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

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2351572460
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2351573798
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2351574206
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2351574601
PR Review Comment: https://git.openjdk.org/jdk/pull/26761#discussion_r2351575139