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