Skip to content

Commit 1bfcc27

Browse files
egahlinegahlin
committed
8294931: JFR: Simplify SettingInfo
Reviewed-by: mgronlun
1 parent eb90c4f commit 1bfcc27

File tree

5 files changed

+34
-48
lines changed

5 files changed

+34
-48
lines changed

‎src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java

+11-6
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import jdk.jfr.StackTrace;
4545
import jdk.jfr.Threshold;
4646
import jdk.jfr.events.ActiveSettingEvent;
47-
import jdk.jfr.internal.EventInstrumentation.SettingInfo;
4847
import jdk.jfr.internal.settings.CutoffSetting;
4948
import jdk.jfr.internal.settings.EnabledSetting;
5049
import jdk.jfr.internal.settings.PeriodSetting;
@@ -66,7 +65,7 @@ record NamedControl(String name, Control control) {
6665
private static final Type TYPE_CUTOFF = TypeLibrary.createType(CutoffSetting.class);
6766
private static final Type TYPE_THROTTLE = TypeLibrary.createType(ThrottleSetting.class);
6867

69-
private final ArrayList<SettingInfo> settingInfos = new ArrayList<>();
68+
private final ArrayList<SettingControl> settingControls = new ArrayList<>();
7069
private final ArrayList<NamedControl> namedControls = new ArrayList<>(5);
7170
private final PlatformEventType type;
7271
private final String idName;
@@ -163,7 +162,6 @@ private void defineSetting(Class<? extends SettingControl> settingsClass, Method
163162
try {
164163
Module settingModule = settingsClass.getModule();
165164
Modules.addReads(settingModule, EventControl.class.getModule());
166-
int index = settingInfos.size();
167165
SettingControl settingControl = instantiateSettingControl(settingsClass);
168166
Control c = new Control(settingControl, null);
169167
c.setDefault();
@@ -180,7 +178,7 @@ private void defineSetting(Class<? extends SettingControl> settingsClass, Method
180178
aes.trimToSize();
181179
addControl(settingName, c);
182180
eventType.add(PrivateAccess.getInstance().newSettingDescriptor(settingType, settingName, defaultValue, aes));
183-
settingInfos.add(new SettingInfo(FIELD_SETTING_PREFIX + index, index, null, null, settingControl));
181+
settingControls.add(settingControl);
184182
}
185183
} catch (InstantiationException e) {
186184
// Programming error by user, fail fast
@@ -308,7 +306,14 @@ public String getSettingsId() {
308306
return idName;
309307
}
310308

311-
public List<SettingInfo> getSettingInfos() {
312-
return settingInfos;
309+
/**
310+
* A malicious user must never be able to run a callback in the wrong
311+
* context. Methods on SettingControl must therefore never be invoked directly
312+
* by JFR, instead use jdk.jfr.internal.Control.
313+
*
314+
* The returned list is only to be used inside EventConfiguration
315+
*/
316+
public List<SettingControl> getSettingControls() {
317+
return settingControls;
313318
}
314319
}

‎src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java

+13-26
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,10 @@
6060
*/
6161
public final class EventInstrumentation {
6262

63-
record SettingInfo(String fieldName, int index, Type paramType, String methodName, SettingControl settingControl) {
64-
/**
65-
* A malicious user must never be able to run a callback in the wrong
66-
* context. Methods on SettingControl must therefore never be invoked directly
67-
* by JFR, instead use jdk.jfr.internal.Control.
68-
*/
69-
public SettingControl settingControl() {
70-
return this.settingControl;
71-
}
63+
record SettingInfo(Type paramType, String methodName) {
7264
}
7365

74-
record FieldInfo(String fieldName, String fieldDescriptor, String internalClassName) {
66+
record FieldInfo(String name, String descriptor) {
7567
}
7668

7769
public static final String FIELD_EVENT_THREAD = "eventThread";
@@ -136,8 +128,8 @@ record FieldInfo(String fieldName, String fieldDescriptor, String internalClassN
136128
public static Method findStaticCommitMethod(ClassNode classNode, List<FieldInfo> fields) {
137129
StringBuilder sb = new StringBuilder();
138130
sb.append("(");
139-
for (FieldInfo v : fields) {
140-
sb.append(v.fieldDescriptor);
131+
for (FieldInfo field : fields) {
132+
sb.append(field.descriptor);
141133
}
142134
sb.append(")V");
143135
Method m = new Method("commit", sb.toString());
@@ -244,10 +236,8 @@ private static List<SettingInfo> buildSettingInfos(Class<?> superClass, ClassNod
244236
Type[] args = Type.getArgumentTypes(m.desc);
245237
if (args.length == 1) {
246238
Type paramType = args[0];
247-
String fieldName = EventControl.FIELD_SETTING_PREFIX + settingInfos.size();
248-
int index = settingInfos.size();
249239
methodSet.add(m.name);
250-
settingInfos.add(new SettingInfo(fieldName, index, paramType, m.name, null));
240+
settingInfos.add(new SettingInfo(paramType, m.name));
251241
}
252242
}
253243
}
@@ -263,10 +253,8 @@ private static List<SettingInfo> buildSettingInfos(Class<?> superClass, ClassNod
263253
if (method.getParameterCount() == 1) {
264254
Parameter param = method.getParameters()[0];
265255
Type paramType = Type.getType(param.getType());
266-
String fieldName = EventControl.FIELD_SETTING_PREFIX + settingInfos.size();
267-
int index = settingInfos.size();
268256
methodSet.add(method.getName());
269-
settingInfos.add(new SettingInfo(fieldName, index, paramType, method.getName(), null));
257+
settingInfos.add(new SettingInfo(paramType, method.getName()));
270258
}
271259
}
272260
}
@@ -285,11 +273,11 @@ private static List<FieldInfo> buildFieldInfos(Class<?> superClass, ClassNode cl
285273
// control in which order they occur and we can add @Name, @Description
286274
// in Java, instead of in native. It also means code for adding implicit
287275
// fields for native can be reused by Java.
288-
fieldInfos.add(new FieldInfo("startTime", Type.LONG_TYPE.getDescriptor(), classNode.name));
289-
fieldInfos.add(new FieldInfo("duration", Type.LONG_TYPE.getDescriptor(), classNode.name));
276+
fieldInfos.add(new FieldInfo("startTime", Type.LONG_TYPE.getDescriptor()));
277+
fieldInfos.add(new FieldInfo("duration", Type.LONG_TYPE.getDescriptor()));
290278
for (FieldNode field : classNode.fields) {
291279
if (!fieldSet.contains(field.name) && isValidField(field.access, Type.getType(field.desc).getClassName())) {
292-
FieldInfo fi = new FieldInfo(field.name, field.desc, classNode.name);
280+
FieldInfo fi = new FieldInfo(field.name, field.desc);
293281
fieldInfos.add(fi);
294282
fieldSet.add(field.name);
295283
}
@@ -303,7 +291,7 @@ private static List<FieldInfo> buildFieldInfos(Class<?> superClass, ClassNode cl
303291
if (!fieldSet.contains(fieldName)) {
304292
Type fieldType = Type.getType(field.getType());
305293
String internalClassName = ASMToolkit.getInternalName(c.getName());
306-
fieldInfos.add(new FieldInfo(fieldName, fieldType.getDescriptor(), internalClassName));
294+
fieldInfos.add(new FieldInfo(fieldName, fieldType.getDescriptor()));
307295
fieldSet.add(fieldName);
308296
}
309297
}
@@ -574,7 +562,7 @@ private void makeInstrumented() {
574562
// stack: [EW] [EW]
575563
methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
576564
// stack: [EW] [EW] [this]
577-
methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), field.fieldName, field.fieldDescriptor);
565+
methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), field.name, field.descriptor);
578566
// stack: [EW] [EW] <T>
579567
EventWriterMethod eventMethod = EventWriterMethod.lookupMethod(field);
580568
invokeVirtual(methodVisitor, TYPE_EVENT_WRITER, eventMethod.asmMethod);
@@ -633,8 +621,8 @@ private void makeInstrumented() {
633621
methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), FIELD_DURATION, "J");
634622
invokeVirtual(methodVisitor, TYPE_EVENT_CONFIGURATION, METHOD_EVENT_CONFIGURATION_SHOULD_COMMIT);
635623
methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail);
636-
int index = 0;
637-
for (SettingInfo si : settingInfos) {
624+
for (int index = 0; index < settingInfos.size(); index++) {
625+
SettingInfo si = settingInfos.get(index);
638626
// if (!settingsMethod(eventConfiguration.settingX)) goto fail;
639627
methodVisitor.visitIntInsn(Opcodes.ALOAD, 0);
640628
if (untypedEventConfiguration) {
@@ -648,7 +636,6 @@ private void makeInstrumented() {
648636
methodVisitor.visitTypeInsn(Opcodes.CHECKCAST, si.paramType().getInternalName());
649637
methodVisitor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, getInternalClassName(), si.methodName, "(" + si.paramType().getDescriptor() + ")Z", false);
650638
methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail);
651-
index++;
652639
}
653640
// return true
654641
methodVisitor.visitInsn(Opcodes.ICONST_1);

‎src/jdk.jfr/share/classes/jdk/jfr/internal/EventWriterMethod.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,16 @@ public Method asASM() {
6767
*
6868
* @return the method
6969
*/
70-
public static EventWriterMethod lookupMethod(FieldInfo v) {
70+
public static EventWriterMethod lookupMethod(FieldInfo field) {
7171
// event thread
72-
if (v.fieldName().equals(EventInstrumentation.FIELD_EVENT_THREAD)) {
72+
if (field.name().equals(EventInstrumentation.FIELD_EVENT_THREAD)) {
7373
return EventWriterMethod.PUT_EVENT_THREAD;
7474
}
7575
for (EventWriterMethod m : EventWriterMethod.values()) {
76-
if (v.fieldDescriptor().equals(m.typeDescriptor)) {
76+
if (field.descriptor().equals(m.typeDescriptor)) {
7777
return m;
7878
}
7979
}
80-
throw new Error("Unknown type " + v.fieldDescriptor());
80+
throw new Error("Unknown type " + field.descriptor());
8181
}
8282
}

‎src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java

+4-10
Original file line numberDiff line numberDiff line change
@@ -188,15 +188,15 @@ private EventConfiguration getConfiguration(Class<? extends jdk.internal.event.E
188188
return Utils.getConfiguration(eventClass);
189189
}
190190

191-
private EventConfiguration newEventConfiguration(EventType eventType, EventControl ec, SettingControl[] settings) {
191+
private EventConfiguration newEventConfiguration(EventType eventType, EventControl ec) {
192192
try {
193193
if (cachedEventConfigurationConstructor == null) {
194-
var argClasses = new Class<?>[] { EventType.class, EventControl.class, SettingControl[].class };
194+
var argClasses = new Class<?>[] { EventType.class, EventControl.class};
195195
Constructor<EventConfiguration> c = EventConfiguration.class.getDeclaredConstructor(argClasses);
196196
SecuritySupport.setAccessible(c);
197197
cachedEventConfigurationConstructor = c;
198198
}
199-
return cachedEventConfigurationConstructor.newInstance(eventType, ec, settings);
199+
return cachedEventConfigurationConstructor.newInstance(eventType, ec);
200200
} catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
201201
throw new InternalError(e);
202202
}
@@ -209,13 +209,7 @@ private EventConfiguration makeConfiguration(Class<? extends jdk.internal.event.
209209
}
210210
EventType eventType = PrivateAccess.getInstance().newEventType(pEventType);
211211
EventControl ec = new EventControl(pEventType, eventClass);
212-
List<SettingInfo> settingInfos = ec.getSettingInfos();
213-
SettingControl[] settings = new SettingControl[settingInfos.size()];
214-
int index = 0;
215-
for (var settingInfo : settingInfos) {
216-
settings[index++] = settingInfo.settingControl();
217-
}
218-
EventConfiguration configuration = newEventConfiguration(eventType, ec, settings);
212+
EventConfiguration configuration = newEventConfiguration(eventType, ec);
219213
PlatformEventType pe = configuration.getPlatformEventType();
220214
pe.setRegistered(true);
221215
// If class is instrumented or should not be instrumented, mark as instrumented.

‎src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ public final class EventConfiguration {
4040
private final SettingControl[] settings;
4141

4242
// Private constructor so user code can't instantiate
43-
private EventConfiguration(EventType eventType, EventControl eventControl, SettingControl[] settings) {
43+
private EventConfiguration(EventType eventType, EventControl eventControl) {
4444
this.eventType = eventType;
4545
this.platformEventType = PrivateAccess.getInstance().getPlatformEventType(eventType);
4646
this.eventControl = eventControl;
47-
this.settings = settings;
47+
this.settings = eventControl.getSettingControls().toArray(new SettingControl[0]);
4848
}
4949

5050
// Class jdk.jfr.internal.PlatformEventType is not

0 commit comments

Comments
 (0)