RFR: 8312132: Add tracking of multiple address spaces in NMT [v46]

Johan Sjölen jsjolen at openjdk.org
Thu Apr 25 10:37:36 UTC 2024


On Tue, 23 Apr 2024 14:54:38 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Remove faulty condition after removing merging
>>  - Add failing test case
>
> src/hotspot/share/nmt/vmatree.hpp line 113:
> 
>> 111:   };
>> 112: 
>> 113:   using VTreap = TreapNode<size_t, IntervalChange, AddressComparator>;
> 
> Why `VTreap` and not `TreapNode`? What does the `V` alone say?

Just needed a short name, switched to `TreapNode`.

> src/hotspot/share/nmt/vmatree.hpp line 139:
> 
>> 137:   }
>> 138: 
>> 139:   SummaryDiff commit_mapping(size_t from, size_t sz, Metadata& metadata) {
> 
> `size_t` or `address` for `from`?

I've been using `size_t` so far to indicate that we are within some file with some offset. I'm not sure that `address` is ever the right choice for `VMATree` as it is a `uchar*`, indicating that it's a directly dereferencable pointer. It's not a huge deal whether we choose `size_t`, `uintptr_t` or `address` for our internal representation IMHO, as long as the external interface (`MemTracker`) correctly indicates what kind of address is expected.

@tstuefe, @gerard-ziemski. This discussion is easily lost in the sea of comments, so pinging you directly here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1579250998
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1579249971


More information about the hotspot-dev mailing list