Skip to content

Commit d873806

Browse files
roberttoyonagaroberttoyonaga
committed
final cleanup
refactor gate formatting fix
1 parent 60be936 commit d873806

File tree

8 files changed

+85
-120
lines changed

8 files changed

+85
-120
lines changed

‎substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/EveryChunkNativeGCPeriodicEvents.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,14 @@
2727
package com.oracle.svm.core.genscavenge;
2828

2929
import com.oracle.svm.core.Uninterruptible;
30-
import com.oracle.svm.core.genscavenge.GCImpl;
3130
import com.oracle.svm.core.heap.GCCause;
3231
import com.oracle.svm.core.jfr.JfrEvent;
3332
import com.oracle.svm.core.jfr.events.ObjectCountEventSupport;
3433
import jdk.jfr.Event;
3534
import jdk.jfr.Name;
3635
import jdk.jfr.Period;
3736

38-
39-
@Name("EveryChunkPeriodGCEvents")
37+
@Name("EveryChunkPeriodicGCEvents")
4038
@Period(value = "everyChunk")
4139
public class EveryChunkNativeGCPeriodicEvents extends Event {
4240

@@ -45,7 +43,7 @@ public static void emit() {
4543
}
4644

4745
@Uninterruptible(reason = "Set and unset should be atomic with invoked GC to avoid races.")
48-
private static void emitObjectCount(){
46+
private static void emitObjectCount() {
4947
if (JfrEvent.ObjectCount.shouldEmit()) {
5048
ObjectCountEventSupport.setShouldSendRequestableEvent(true);
5149
GCImpl.getGCImpl().collectWithoutAllocating(GCCause.JfrObjectCount, true);

‎substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCEventSupport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import com.oracle.svm.core.Uninterruptible;
3636
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
3737
import com.oracle.svm.core.feature.InternalFeature;
38-
import com.oracle.svm.core.genscavenge.EveryChunkNativeGCPeriodicEvents;
3938
import com.oracle.svm.core.heap.GCCause;
4039
import com.oracle.svm.core.jfr.HasJfrSupport;
4140
import com.oracle.svm.core.jfr.JfrEvent;
@@ -139,6 +138,7 @@ private int popPhase() {
139138
assert currentPhase > 0;
140139
return --currentPhase;
141140
}
141+
142142
public static RuntimeSupport.Hook startupHook() {
143143
return isFirstIsolate -> {
144144
FlightRecorder.addPeriodicEvent(EveryChunkNativeGCPeriodicEvents.class, EveryChunkNativeGCPeriodicEvents::emit);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrTypeRepository.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ private TypeInfo collectTypeInfo(boolean flushpoint) {
102102
return typeInfo;
103103
}
104104

105-
boolean isClassGenerated(Class<?> clazz){
106-
return clazz != null && clazz.getPackage() != null && clazz.getPackage().getName().equals("jdk.internal.reflect")
107-
&& clazz.getName().contains("GeneratedSerializationConstructorAccessor");
105+
boolean isClassGenerated(Class<?> clazz) {
106+
return clazz != null && clazz.getPackage() != null && clazz.getPackage().getName().equals("jdk.internal.reflect") && clazz.getName().contains("GeneratedSerializationConstructorAccessor");
108107
}
108+
109109
private void visitClass(TypeInfo typeInfo, Class<?> clazz) {
110110
if ((clazz != null && addClass(typeInfo, clazz))) {
111111
visitPackage(typeInfo, clazz.getPackage(), clazz.getModule(), isClassGenerated(clazz));
@@ -114,11 +114,6 @@ private void visitClass(TypeInfo typeInfo, Class<?> clazz) {
114114
}
115115
}
116116

117-
// TODO need to add two module entries in map if generated class ( 1pkg --> 2 modules)... WHY?
118-
// TODO can 1 module -- > 2 classLoaders ? [probably not.]
119-
// TODO can 1 class -- > 2 packages ? [NO]
120-
//TODO: is there only one "unnamed module" if so we can lazily add it to the module map when we see it [but theres no point because its only referenced from the packages entries, not the classes]
121-
// TODO: only use packageInfo containing unnamed module if no non-generated classes are encountered
122117
private void visitPackage(TypeInfo typeInfo, Package pkg, Module module, boolean generated) {
123118
if (pkg != null && addPackage(typeInfo, pkg, module, generated)) {
124119
visitModule(typeInfo, module);
@@ -259,7 +254,7 @@ private boolean isClassVisited(TypeInfo typeInfo, Class<?> clazz) {
259254
private boolean addPackage(TypeInfo typeInfo, Package pkg, Module module, boolean generated) {
260255
if (isPackageVisited(typeInfo, pkg)) {
261256
Module cached = (flushedPackages.containsKey(pkg.getName()) ? flushedPackages.get(pkg.getName()).module : typeInfo.packages.get(pkg.getName()).module);
262-
if (cached.isNamed() || !module.isNamed()){
257+
if (cached.isNamed() || !module.isNamed()) {
263258
assert module == cached || (generated && !module.isNamed());
264259
return false;
265260
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ObjectCountEventSupport.java

Lines changed: 34 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,16 @@
2727
package com.oracle.svm.core.jfr.events;
2828

2929
import com.oracle.svm.core.Uninterruptible;
30-
import com.oracle.svm.core.UnmanagedMemoryUtil;
3130
import com.oracle.svm.core.heap.Heap;
3231
import com.oracle.svm.core.heap.ObjectVisitor;
33-
import com.oracle.svm.core.heap.VMOperationInfos;
3432
import com.oracle.svm.core.hub.DynamicHub;
3533
import com.oracle.svm.core.hub.DynamicHubSupport;
3634
import com.oracle.svm.core.hub.LayoutEncoding;
3735
import com.oracle.svm.core.jfr.HasJfrSupport;
3836
import com.oracle.svm.core.jfr.JfrEvent;
3937
import com.oracle.svm.core.jfr.SubstrateJVM;
40-
import com.oracle.svm.core.jfr.events.ObjectCountEvents;
4138
import com.oracle.svm.core.jfr.utils.PointerArray;
4239
import com.oracle.svm.core.jfr.utils.PointerArrayAccess;
43-
import com.oracle.svm.core.thread.NativeVMOperation;
44-
import com.oracle.svm.core.thread.NativeVMOperationData;
45-
import com.oracle.svm.core.thread.VMOperation.SystemEffect;
4640
import com.oracle.svm.core.thread.VMOperation;
4741
import com.oracle.svm.core.util.VMError;
4842

@@ -55,42 +49,34 @@
5549
import org.graalvm.nativeimage.c.struct.SizeOf;
5650
import org.graalvm.nativeimage.impl.UnmanagedMemorySupport;
5751

58-
import org.graalvm.word.Pointer;
5952
import org.graalvm.word.PointerBase;
60-
import org.graalvm.word.WordFactory;
6153

6254
public class ObjectCountEventSupport {
63-
private final static ObjectCountVisitor objectCountVisitor = new ObjectCountVisitor();
64-
// private static final ObjectCountOperation objectCountOperation = new ObjectCountOperation();
65-
private static double cutoffPercentage = 0.005; // *** don't need user input for this. Hotspot simply hardcodes this
66-
private static long totalSize;
67-
68-
// *** should be volatile because periodic thread writes it and VM op thread reads it
55+
private static final double CUTOFF_PERCENTAGE = 0.005;
56+
private static final ObjectCountVisitor objectCountVisitor = new ObjectCountVisitor();
6957
private static volatile boolean shouldSendRequestableEvent = false;
7058

7159
@Platforms(HOSTED_ONLY.class)
7260
ObjectCountEventSupport() {
7361
}
7462

7563
@Uninterruptible(reason = "Set and unset should be atomic with invoked GC.", callerMustBe = true)
76-
public static void setShouldSendRequestableEvent(boolean value){
64+
public static void setShouldSendRequestableEvent(boolean value) {
7765
shouldSendRequestableEvent = value;
7866
}
7967

80-
public static void emitEvents(int gcId, long startTicks){
68+
public static void emitEvents(int gcId, long startTicks) {
8169
if (HasJfrSupport.get() && shouldEmitEvents()) {
82-
emitEvents0(gcId, startTicks);
83-
// if (shouldSendRequestableEvent){
84-
// shouldSendRequestableEvent = false;
85-
// }
70+
emitEvents0(gcId, startTicks);
8671
}
8772
}
8873

89-
/** ShouldEmit will be checked again later. This is merely an optimization.*/
74+
/** ShouldEmit will be checked again later. This is merely an optimization. */
9075
@Uninterruptible(reason = "Caller of JfrEvent#shouldEmit must be uninterruptible.")
91-
private static boolean shouldEmitEvents(){
76+
private static boolean shouldEmitEvents() {
9277
return shouldSendRequestableEvent || JfrEvent.ObjectCountAfterGC.shouldEmit();
9378
}
79+
9480
private static void emitEvents0(int gcId, long startTicks) {
9581
PointerArray objectCounts = StackValue.get(PointerArray.class);
9682
try {
@@ -99,79 +85,34 @@ private static void emitEvents0(int gcId, long startTicks) {
9985
return;
10086
}
10187

102-
// Throw an error, otherwise we'll probably get a segfault later
103-
VMError.guarantee(objectCounts.getSize() == initialCapacity, "init not done properly");
104-
countObjects(objectCounts);
88+
long totalSize = visitObjects(objectCounts);
10589

10690
for (int i = 0; i < objectCounts.getSize(); i++) {
107-
emitForTypeId(i, objectCounts, gcId, startTicks);
91+
emitForTypeId(i, objectCounts, gcId, startTicks, totalSize);
10892
}
10993
} finally {
11094
PointerArrayAccess.freeData(objectCounts);
11195
}
11296
}
11397

114-
private static void emitForTypeId(int typeId, PointerArray objectCounts, int gcId, long startTicks){
98+
private static void emitForTypeId(int typeId, PointerArray objectCounts, int gcId, long startTicks, long totalSize) {
11599
ObjectCountData objectCountData = (ObjectCountData) PointerArrayAccess.get(objectCounts, typeId);
116-
if (objectCountData.isNonNull() && objectCountData.getSize() / (double) totalSize > cutoffPercentage) {
117-
VMError.guarantee(objectCountData.getSize() > 0 && objectCountData.getTraceId() >0 && objectCountData.getSize()>0, "size should be > 0 if count is > 0");
100+
if (objectCountData.isNonNull() && objectCountData.getSize() / (double) totalSize > CUTOFF_PERCENTAGE) {
101+
VMError.guarantee(objectCountData.getSize() > 0 && objectCountData.getTraceId() > 0 && objectCountData.getSize() > 0, "size should be > 0 if count is > 0");
118102
if (shouldSendRequestableEvent) {
119103
ObjectCountEvents.emit(JfrEvent.ObjectCount, startTicks, objectCountData.getTraceId(), objectCountData.getCount(), objectCountData.getSize(), gcId);
120104
}
121105
ObjectCountEvents.emit(JfrEvent.ObjectCountAfterGC, startTicks, objectCountData.getTraceId(), objectCountData.getCount(), objectCountData.getSize(), gcId);
122106
}
123107
}
124108

125-
private static void countObjects(PointerArray objectCounts) {
109+
private static long visitObjects(PointerArray objectCounts) {
126110
assert VMOperation.isGCInProgress();
127-
// int size = SizeOf.get(ObjectCountVMOperationData.class);
128-
// ObjectCountVMOperationData vmOpData = StackValue.get(size);
129-
// UnmanagedMemoryUtil.fill((Pointer) vmOpData, WordFactory.unsigned(size), (byte) 0);
130-
131-
// vmOpData.setObjectCounts(objectCounts);
132-
// objectCountOperation.enqueue(vmOpData);
133-
// objectCountVisitor.initialize(vmOpData.getObjectCounts());
134111
objectCountVisitor.initialize(objectCounts);
135-
totalSize = 0; // compute anew each time we operate
136112
Heap.getHeap().walkImageHeapObjects(objectCountVisitor);
137-
138-
int sizeSum = 0;
139-
int countSum = 0;
140-
for (int i = 0; i < objectCounts.getSize(); i++) {
141-
ObjectCountData objectCountData = (ObjectCountData) PointerArrayAccess.get(objectCounts, i);
142-
if (objectCountData.isNull()) {
143-
continue;
144-
}
145-
countSum += objectCountData.getCount();
146-
sizeSum += objectCountData.getSize();
147-
VMError.guarantee(objectCountData.getSize() > 0, "size should be > 0 if count is > 0");
148-
}
149-
150-
VMError.guarantee(countSum > 0, "countSum should be >0");
151-
VMError.guarantee(sizeSum > 0, "sizeSum should be >0");
152-
153-
int typeId = DynamicHub.fromClass(String.class).getTypeID();
154-
ObjectCountData stringOcd = objectCounts.getData().addressOf(typeId).read();
155-
VMError.guarantee(stringOcd.getCount() > 0, "should have more than 1 String in heap");
156-
VMError.guarantee(stringOcd.getSize() > 0, "string size should be positive");
113+
return objectCountVisitor.getTotalSize();
157114
}
158115

159-
// private static class ObjectCountOperation extends NativeVMOperation {
160-
// @Platforms(HOSTED_ONLY.class)
161-
// ObjectCountOperation() {
162-
// super(VMOperationInfos.get(ObjectCountOperation.class, "JFR count objects", SystemEffect.SAFEPOINT));
163-
// }
164-
//
165-
// @Override
166-
// protected void operate(NativeVMOperationData data) {
167-
// ObjectCountVMOperationData objectCountVMOperationData = (ObjectCountVMOperationData) data;
168-
// objectCountVisitor.initialize(objectCountVMOperationData.getObjectCounts());
169-
// totalSize = 0; // compute anew each time we operate
170-
// Heap.getHeap().walkImageHeapObjects(objectCountVisitor);
171-
// }
172-
// }
173-
174-
175116
private static boolean initializeObjectCountData(PointerArray pointerArray, int idx, Object obj) {
176117
ObjectCountData objectCountData = ImageSingletons.lookup(UnmanagedMemorySupport.class).malloc(SizeOf.unsigned(ObjectCountData.class));
177118
if (objectCountData.isNull()) {
@@ -184,33 +125,37 @@ private static boolean initializeObjectCountData(PointerArray pointerArray, int
184125
return true;
185126
}
186127

187-
/** JFR epoch will not change before associated ObjectCount event is committed because this code runs within a
188-
* GC safepoint.*/ //*** epoch cannot change bc we already are in a safepoint // TODO revisit this logic
128+
/**
129+
* It's ok to ge the trace ID here because JFR epoch will not change before jdk.ObjectCount
130+
* events are committed.
131+
*/
189132
@Uninterruptible(reason = "Caller of SubstrateJVM#getClassId must be uninterruptible.")
190-
private static long getTraceId(Class<?> c){
133+
private static long getTraceId(Class<?> c) {
191134
assert VMOperation.isGCInProgress();
192135
return SubstrateJVM.get().getClassId(c);
193136
}
194137

195138
private static class ObjectCountVisitor implements ObjectVisitor {
196-
PointerArray objectCounts;
139+
private PointerArray objectCounts;
140+
private long totalSize;
197141

198142
@Platforms(HOSTED_ONLY.class)
199143
ObjectCountVisitor() {
200144
}
201145

202146
public void initialize(PointerArray objectCounts) {
203147
this.objectCounts = objectCounts;
148+
this.totalSize = 0;
204149
}
205150

206151
@Override
207-
public boolean visitObject(Object obj) { // *** Can't allocate in here no matter what.
152+
public boolean visitObject(Object obj) {
208153
assert VMOperation.isGCInProgress();
209154
DynamicHub hub = DynamicHub.fromClass(obj.getClass());
210155
int typeId = hub.getTypeID();
211-
VMError.guarantee(typeId < objectCounts.getSize(), "Should not encounter a typeId out of scope of the array");
156+
assert typeId < objectCounts.getSize();
212157

213-
// create an ObjectCountData for this typeID if one doesn't already exist
158+
// Create an ObjectCountData for this typeID if one doesn't already exist
214159
ObjectCountData objectCountData = objectCounts.getData().addressOf(typeId).read();
215160
if (objectCountData.isNull()) {
216161
if (!initializeObjectCountData(objectCounts, typeId, obj)) {
@@ -231,23 +176,20 @@ public boolean visitObject(Object obj) { // *** Can't allocate in here no matter
231176
return true;
232177
}
233178

234-
/** Safe to call from interruptible code because GC should not touch this object again before we
235-
* are done with it.*/ // *** no allocation and already in a GC safepoint // TODO revisit this logic
179+
/**
180+
* Caller can be interruptible code because GC should not touch this object again before we
181+
* are done with it.
182+
*/
236183
@Uninterruptible(reason = "Caller of LayoutEncoding#getSizeFromObject must be uninterruptible.")
237184
private long uninterruptibleGetSize(Object obj) {
238185
assert VMOperation.isGCInProgress();
239186
return LayoutEncoding.getSizeFromObject(obj).rawValue();
240187
}
241-
}
242188

243-
// @RawStructure
244-
// private interface ObjectCountVMOperationData extends NativeVMOperationData {
245-
// @RawField
246-
// PointerArray getObjectCounts();
247-
//
248-
// @RawField
249-
// void setObjectCounts(PointerArray value);
250-
// }
189+
public long getTotalSize() {
190+
return totalSize;
191+
}
192+
}
251193

252194
@RawStructure
253195
public interface ObjectCountData extends PointerBase {

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ObjectCountEvents.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@
3535
import com.oracle.svm.core.thread.VMOperation;
3636
import org.graalvm.nativeimage.StackValue;
3737

38-
/** This class is used for both jdk.ObjectCount and jdk.ObjectCountAfterGC since they contain identical information.*/
38+
/**
39+
* This class is used for both jdk.ObjectCount and jdk.ObjectCountAfterGC since they contain
40+
* identical information.
41+
*/
3942
public class ObjectCountEvents {
4043
public static void emit(JfrEvent eventType, long startTick, long traceId, long count, long size, int gcId) {
4144
assert VMOperation.isInProgressAtSafepoint();
@@ -45,8 +48,8 @@ public static void emit(JfrEvent eventType, long startTick, long traceId, long c
4548
}
4649

4750
@Uninterruptible(reason = "Accesses a JFR buffer.")
48-
public static void emit0(JfrEvent eventType, long startTick, long traceId, long count, long size, int gcId) {
49-
if (eventType.shouldEmit()){
51+
public static void emit0(JfrEvent eventType, long startTick, long traceId, long count, long size, int gcId) {
52+
if (eventType.shouldEmit()) {
5053
JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
5154
JfrNativeEventWriterDataAccess.initializeThreadLocalNativeBuffer(data);
5255
JfrNativeEventWriter.beginSmallEvent(data, eventType);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/utils/PointerArray.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
/*
2+
* Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
3+
* Copyright (c) 2023, 2023, Red Hat Inc. All rights reserved.
4+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
5+
*
6+
* This code is free software; you can redistribute it and/or modify it
7+
* under the terms of the GNU General Public License version 2 only, as
8+
* published by the Free Software Foundation. Oracle designates this
9+
* particular file as subject to the "Classpath" exception as provided
10+
* by Oracle in the LICENSE file that accompanied this code.
11+
*
12+
* This code is distributed in the hope that it will be useful, but WITHOUT
13+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
14+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
15+
* version 2 for more details (a copy is included in the LICENSE file that
16+
* accompanied this code).
17+
*
18+
* You should have received a copy of the GNU General Public License version
19+
* 2 along with this work; if not, write to the Free Software Foundation,
20+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
21+
*
22+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
23+
* or visit www.oracle.com if you need additional information or have any
24+
* questions.
25+
*/
26+
127
package com.oracle.svm.core.jfr.utils;
228

329
import org.graalvm.nativeimage.c.struct.RawStructure;

0 commit comments

Comments
 (0)