On Mon, 26 Oct 2020 07:40:50 GMT, Lin Zang <lz...@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
