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

Afshin Zafari azafari at openjdk.org
Mon Dec 9 10:45:36 UTC 2024


On Fri, 6 Dec 2024 15:50:57 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   a missed change in a shenandoah file.
>
> src/hotspot/share/nmt/regionsTree.hpp line 94:
> 
>> 92:   template<typename F>
>> 93:   void visit_committed_regions(position start, size_t size, F func) {
>> 94:     size_t end = start + size + 1;
> 
>> Yes it is missed.
> In the visit_range_in_order code, the variable cmp_to is never checked against 0 (cmp_to == 0).
> 
> You are referring to [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/nmt/nmtTreap.hpp#L380) right? I think that is actually ok without the +1. The upper boundary, "end", is outside of the range of the region, so probably should not be checked. For example if the region's starting address is 0 and has a size of 10, the last address within the region is 9. "End" in this case is 10 (0+10), and is out of bounds. If we made "end" = 0 + 10 + 1 = 11, then we would be including 10 in the checked range, which isn't right.

The `VMATree` holds the region `[A,B)` as two nodes with `A` and `B`. So, in your example the nodes in the tree are `0` and `10`. Looking for `< 10` does not find `B`, but `< 11` does..
The case with `end == 11` has no problem also when two regions are adjacent. If the next region has size `10`, for example,  then the nodes are `0`, `10` and `20`. `< 11` will find `10( = B)` and not `20`.

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

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


More information about the hotspot-runtime-dev mailing list