RFR: 8340103: Add internal set_flag function to VMATree [v11]

Johan Sjölen jsjolen at openjdk.org
Wed Nov 6 10:15:32 UTC 2024


On Mon, 4 Nov 2024 15:12:40 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix incorrect merge
>
> src/hotspot/share/nmt/vmatree.cpp line 250:
> 
>> 248:   // If end < from + sz then there are multiple ranges for which to set the flag.
>> 249:   while (end < from + size) {
>> 250:     range = _tree.find_enclosing_range(from);
> 
> This is a bit of a brain teaser.
> 
> I first wondered why we needed to find the "from" node again, since we already have it. But the last `register_mapping` call can actually have coalesced ranges and either the old `from` node or the old `to` node can have disappeared. 
> 
> So, we have two cases: 
> - the old "to" node is still there, in which case we could just read its RegionData state and use that for the next segment (modulo tag)
> - the old "to" node disappeared due to merging (meaning, tagging the last segment causes it to coalesce with the next segment), in which case there is nothing to do, since this segment is already correctly tagged.
> 
> But I am not 100% sure. So, re-querying the range from the treap is probably the safest way to go. 
> 
> But can you please add a comment stating that register_mapping may have modified the tree structure and caused segments to coalesce, just invalidating the old `range` ?

Comment added.

> test/hotspot/gtest/nmt/test_vmatree.cpp line 347:
> 
>> 345:     EXPECT_EQ(20, diff.tag[i(mtClassShared)].commit);
>> 346:   }
>> 347: }
> 
> I would like to see the following cases explicitly tested:
> 1) set a tag over a range spanning multiple existing segments with the same stack but different tags - here, I would like to see that set_tag causes the tree to coalesce these different ranges to a single range
> 2) set a tag over a range spanning multiple existing segments with different stacks and different tags - here, I would like to see that the original ranges stay intact, including their stack information, only the tags should now all be the same
> 3) set a tag within a range - here, I would like to see three ranges now
> 
> for 1, also for mismatched starts and ends where either start or end are in the middle of an existing range, which should cause the tree to split that range

Tests added, and they all passed! I really didn't think that case 3 would work, but sometimes I write good code I guess.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20994#discussion_r1830730462
PR Review Comment: https://git.openjdk.org/jdk/pull/20994#discussion_r1830727484


More information about the hotspot-runtime-dev mailing list