RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion'

Chris Plummer cjplummer at openjdk.org
Sat Apr 20 03:15:37 UTC 2024


On Sat, 20 Apr 2024 02:04:20 GMT, Lei Zaakjyu <duke at openjdk.org> wrote:

> follow up 8267941

Functionally the SA changes look good, but I pointed out a few other places that you might also want to consider doing renames for.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerFinder.java line 123:

> 121:       if (heap instanceof G1CollectedHeap) {
> 122:         G1CollectedHeap g1 = (G1CollectedHeap)heap;
> 123:         loc.hr = g1.heapRegionForAddress(a);

"g1HeapRegionForAddress"?

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerFinder.java line 126:

> 124:         // We don't assert that loc.hr is not null like we do for the SerialHeap. This is
> 125:         // because heap.isIn(a) can return true if the address is anywhere in G1's mapped
> 126:         // memory, even if that area of memory is not in use by a G1 G1HeapRegion. So there

Suggestion:

        // memory, even if that area of memory is not in use by a G1HeapRegion. So there

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerLocation.java line 131:

> 129:   }
> 130: 
> 131:   public G1HeapRegion getHeapRegion() {

Do we want to rename to getG1HeapRegion?

test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java line 62:

> 60:             agent.attach(Integer.parseInt(pid));
> 61:             G1CollectedHeap heap = (G1CollectedHeap)VM.getVM().getUniverse().heap();
> 62:             G1HeapRegion hr = heap.hrm().heapRegionIterator().next();

"g1HeapRegionIterator"?

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

PR Review: https://git.openjdk.org/jdk/pull/18871#pullrequestreview-2013005488
PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573125957
PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573124198
PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573124436
PR Review Comment: https://git.openjdk.org/jdk/pull/18871#discussion_r1573125643


More information about the serviceability-dev mailing list