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

Gerard Ziemski gziemski at openjdk.org
Wed Sep 25 16:03:36 UTC 2024


On Wed, 18 Sep 2024 10:39:46 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi!
>> 
>> The old VirtualMemoryTracker has a method set_reserved_region_type(address, flag). We implement this for the new VMATree implementation by altering the signature slightly to set_reserved_region_type(address, size, flag). This simplifies the implementation greatly for our new data structure and leads to trivial changes for the callers (all callers already know the size).
>> 
>> This PR implements the internal implementation along with tests, but does not change any callers.
>> 
>> I also do a few cleanups:
>> 
>> - Change `Node` to `TreeNode` in tests, we've seen build failures because of this (probably a precompiled headers issue)
>> - Add a few `print_on` methods for  easy debugging
>> - Add a `size` alias, it was a bit confusing that some functions took an argument `position sz`, so changed that to `size sz` 
>> 
>> Thanks.
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   apply -> add

Changes requested by gziemski (Committer).

src/hotspot/share/nmt/vmatree.cpp line 205:

> 203:   visit_in_order([&](TreapNode* current) {
> 204:     out->print(SIZE_FORMAT " (%s) - %s - ", current->key(), NMTUtil::tag_to_name(current->val().out.mem_tag()),
> 205:                statetype_to_string(current->val().out.type()));

Could we consider here adding a utility function:


  IntervalState getState(TreapNode* node) {
    return node->val().out;
  }


to simplify from:


    out->print(SIZE_FORMAT " (%s) - %s - ", current->key(), NMTUtil::tag_to_name(current->val().out.mem_tag()),
                statetype_to_string(current->val().out.type()));


to


    out->print(SIZE_FORMAT " (%s) - %s - ", current->key(), NMTUtil::tag_to_name(getState(current).mem_tag()),
                statetype_to_string(getState(current).type()));


and all the other from:

`    stB.out = leqA_n->val().out;
`

to:

`    stB.out = getState(leqA_n);
`

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

PR Review: https://git.openjdk.org/jdk/pull/20994#pullrequestreview-2328724857
PR Review Comment: https://git.openjdk.org/jdk/pull/20994#discussion_r1775504828


More information about the hotspot-runtime-dev mailing list