On Sat, 20 Apr 2024 02:04:20 GMT, Lei Zaakjyu <d...@openjdk.org> wrote:
> follow up 8267941 So much scrolling :) Looks good. Just a few very minor nits for which I don't need to re-review. src/hotspot/share/cds/archiveHeapWriter.cpp line 90: > 88: > 89: guarantee(UseG1GC, "implementation limitation"); > 90: guarantee(MIN_GC_REGION_ALIGNMENT <= > /*G1*/G1HeapRegion::min_region_size_in_words() * HeapWordSize, "must be"); The "/*G1*/" comment should be removed. src/hotspot/share/gc/g1/g1ConcurrentMark.hpp line 761: > 759: > 760: // Region this task is scanning, null if we're not scanning any > 761: G1HeapRegion* _curr_region; Adjust indentation of member name to (re)match those nearby. src/hotspot/share/gc/g1/g1ConcurrentMarkBitMap.hpp line 39: > 37: class G1CMTask; > 38: class G1ConcurrentMark; > 39: class G1HeapRegion; With this forward declaration rename being the only change, I wonder if the declaration is even needed. Try deleting it, but keep if removing produces non-trivial effects elsewhere. src/hotspot/share/gc/g1/g1FullGCCompactionPoint.hpp line 40: > 38: G1FullCollector* _collector; > 39: G1HeapRegion* _current_region; > 40: HeapWord* _compaction_top; Tidy indentation of `_compaction_top`. My preference would be to remove extra whitespace before it, rather than adding to (re)line up with new position of `_current_region`. src/hotspot/share/gc/g1/g1OopClosures.hpp line 33: > 31: #include "oops/markWord.hpp" > 32: > 33: class G1HeapRegion; With no other changes in this file, maybe this forward declaration isn't needed at all? But keep for now if removal leads to non-trivial additional changes. ------------- Marked as reviewed by kbarrett (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18871#pullrequestreview-2013013903 PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573137157 PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573138750 PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573139162 PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573140388 PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573141541
