RFR: 8252103: Parallel heap inspection for ParallelScavengeHeap [v9]
Thomas Schatzl
tschatzl at openjdk.java.net
Mon Nov 2 11:05:00 UTC 2020
On Fri, 30 Oct 2020 19:27:01 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
>> Lin Zang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into jmap-par
>> - Merge remote-tracking branch 'upstream/master' into jmap-par
>> - update the return type of iterable_blocks to size_t
>> - cast iterable_blocks to int in claim_and_get_block()
>> - fix constant coding style and do code refine
>> - Refine HeapBlockClaimer implementation
>> - 8252103: Parallel heap inspection for ParallelScavengeHeap
>>
>> - Parallel heap iteration support for PSS
>> - JBS: https://bugs.openjdk.java.net/browse/JDK-8252103
>
> Changes requested by tschatzl (Reviewer).
> Hi @tschatzl, Thanks for your reviewing. I think the biggest problem you mentioned is that the type of _claimed_index and old_gen()->iterable_blocks() is not match (conversion from unsigned to signed). As I cannot change _claim_index to be unsigned because it use -1 and -2 for identifing youngGen, do you think that changing "iterable_blocks()" to be a signed value is reasonable? and I will also change all these types to be 64bit signed value to avoid auto widening.
Yes, the signed/unsigned changes are the largest issue.
You could use the mentioned `ssize_t` as an automatically widening signed integer type. However you could also use `(size_t)-1/-2` (i.e. `MAX_SIZE_T/MAX_SIZE_T-1` or whatever they are called) as identifiers for young and survivor. There seems to be no difference in using either since the code only does direct comparisons.
This works because you can't have these values for regular old gen blocks (as block size is 1M, so the maximum value for a regular block is 2^(64-20) - you can STATIC_ASSERT that block size is big enough to not cover these special values btw).
Or alternatively, `(u)intptr_t` as its range is defined to cover all possible pointer values (which is what we care about) and might be the best fit.
I'm good with all of these options.
-------------
PR: https://git.openjdk.java.net/jdk/pull/25
More information about the hotspot-gc-dev
mailing list