RFR: 8337217: Port VirtualMemoryTracker to use VMATree [v8]

[Afshin Zafari]

On Tue, 10 Dec 2024 14:46:34 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

>> Your point on `base` is correct. `prev.position` can be used instead.
>> 
>> I agree that there is ambiguity in the method name and its args. This method SHOULD be called for a `ReservedMemoryRegion [base, base + size)`. This is implicitly assumed. The signature as `visit_committed_regions(ReservedMemoryRegion& rgn, F func)` is more declarative then.
>> 
>> With the above assumption, if two committed regions are adjacent with different `MemTag`, they certainly belong to different `ReservedMemoryRegion`s, one for CDS with `mtClassShared` and another for stack with `mtThreadStack` MemTag. This method doesn't care the `MemTag`s because all the sub-regions within the `ReservedMemoryRegion [base, base + size)` are assumed to have the same `MemTag`.
>
> Ok that makes sense to me if `visit_committed_regions` is only ever called on a single ReservedRegion.  But if that assumption is true, then isn't it also true that we should never encounter two adjacent committed regions (they should already be coalesced)? 
> 
> So then maybe you don't need the extra handling such as tracking `comm_size` or checking `if (!curr.is_committed_begin())`. Maybe you could just directly have something like:
> 
> if (prev.is_valid() && prev.is_committed_begin()) {
>     CommittedMemoryRegion cmr((address) prev.position(), curr.distance_from(prev), st);
> ...
> }

Very good comment. Thanks. Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1886533184