[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR: 8252103: Parallel heap inspection for ParallelScavengeHeap
From:       "Hohensee, Paul" <hohensee () amazon ! com>
Date:       2020-09-28 22:39:38
Message-ID: 25A4DC80-74A8-4C99-AEF4-7E989317B18A () amazon ! com
[Download RAW message or body]

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@openjdk.java.net on behalf of lzang@openjdk.java.net> wrote:

    On Sun, 6 Sep 2020 01:13:48 GMT, Lin Zang <lzang@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


[prev in list] [next in list] [prev in thread] [next in thread]