RFR: 8252103: Parallel heap inspection for ParallelScavengeHeap

Hohensee, Paul hohensee at amazon.com
Mon Sep 28 22:39:38 UTC 2020


I'm not a GC specialist, but your approach looks reasonable to me.

parallelScavengeHeap.cpp:

"less that 1 workers" -> "less that 1 worker"

psOldGen.cpp:

">=2" -> ">= 2"

"thread_num-2 worker" -> "(thread_num -2) workers"

"cross blocks" -> "crosses blocks"

"that the object start address locates in the related block" -> "that owns the block within which the object starts"

blocks_iterate_parallel() relies on BLOCK_SIZE being an integral multiple of a start_array() block (i.e., ObjectStartArray:: block_size). I'd add an assert to that effect.

The comment on the definition of BLOCK_SIZE is "64 KB", but you're doing pointer arithmetic in terms of HeapWords, which means the actual byte value is either 256KB or 512KB for 32- and 64-bit respectively. You could use

const int BLOCK_SIZE = (64 * 1024 / HeapWordSize);

"// There is objects" -> "There are objects"

"reture" -> "return"

", here only focus objects" -> ". Here only focus on objects"

It's a matter of taste, but imo the loop in blocks_iterate_parallel() would be more clear as

for (HeapWord* p = start; p < end; p += oop(p)->size()) {
    cl->do_object(oop(p));
}

psOldGen.hpp:

blocks_iterate_parallel() is pure implementation and I don't see a reference outside psOldGen.cpp. So, it should be private, not public.

psYoungGen.cpp:

"<=1" -> "<= 1"

Thanks,
Paul

On 9/11/20, 12:29 AM, "hotspot-gc-dev on behalf of Lin Zang" <hotspot-gc-dev-retn at openjdk.java.net on behalf of lzang at openjdk.java.net> wrote:

    On Sun, 6 Sep 2020 01:13:48 GMT, Lin Zang <lzang at openjdk.org> wrote:

    > - Parallel heap iteration support for PSS
    > - JBS:  https://bugs.openjdk.java.net/browse/JDK-8252103

    Dear All,
      May I ask your help to review this PR? Thanks!
    -Lin

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

    PR: https://git.openjdk.java.net/jdk/pull/25



More information about the serviceability-dev mailing list