RFR: 8252103: Parallel heap inspection for ParallelScavengeHeap [v5]

Stefan Johansson sjohanss at openjdk.java.net
Wed Oct 28 11:04:26 UTC 2020


On Mon, 26 Oct 2020 07:40:50 GMT, Lin Zang <lzang at openjdk.org> wrote:

>> - Parallel heap iteration support for PSS
>> - JBS:  https://bugs.openjdk.java.net/browse/JDK-8252103
>
> Lin Zang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refine HeapBlockClaimer implementation

Thanks for the update, some additional comments, but just minor things.

I did some basic local testing and I've also started some tests to build on multiple platforms, once the review is complete I will do a more thorough test run to make sure the code is exercised on all platforms as well.

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 588:

> 586:   return new PSScavengeParallelObjectIterator(thread_num);
> 587: }
> 588: 

No need to pass down thread_num to `PSScavengeParallelObjectIterator` because it is not needed anymore. This way we can also remove the `_thread_num` member.

src/hotspot/share/gc/parallel/psOldGen.hpp line 56:

> 54: 
> 55:   // Block size for parallel iteration
> 56:   static const size_t _iterate_block_size = 1024 * 1024;

Constants are named without _ and using camel case. I think you can also add to the comment that the size is in bytes:
Suggestion:

  // Block size in bytes for parallel iteration
  static const size_t IterateBlockSize = 1024 * 1024;

Also remember to update the comments referring to the constant :)

src/hotspot/share/gc/parallel/psOldGen.hpp line 169:

> 167:   void object_iterate(ObjectClosure* cl) { object_space()->object_iterate(cl); }
> 168:   uint iterable_blocks() {
> 169:     return (object_space()->used_in_bytes() + _iterate_block_size -1) / _iterate_block_size;

Space before 1 in `-1`.

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

Changes requested by sjohanss (Reviewer).

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


More information about the serviceability-dev mailing list