From b263292a75de14b39852c3d2fc73deb3fefabb9a Mon Sep 17 00:00:00 2001 From: Erik Gahlin Date: Thu, 3 Apr 2025 11:07:52 +0000 Subject: [PATCH] 8353484: JFR: Simplify EventConfiguration Reviewed-by: mgronlun --- .../jdk/jfr/internal/MetadataRepository.java | 42 ++++++----------- .../jdk/jfr/internal/SettingsManager.java | 4 +- .../internal/event/EventConfiguration.java | 46 ++++--------------- .../jdk/jfr/internal/event/EventWriter.java | 4 +- 4 files changed, 26 insertions(+), 70 deletions(-) diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java index 951142e2493..5c44f2416cf 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java @@ -44,6 +44,7 @@ import jdk.jfr.Event; import jdk.jfr.EventType; import jdk.jfr.Name; import jdk.jfr.Period; +import jdk.jfr.SettingControl; import jdk.jfr.ValueDescriptor; import jdk.jfr.internal.consumer.RepositoryFiles; import jdk.jfr.internal.event.EventConfiguration; @@ -59,7 +60,6 @@ public final class MetadataRepository { private final Map nativeControls = LinkedHashMap.newHashMap(150); private final SettingsManager settingsManager = new SettingsManager(); private final HiddenWait threadSleeper = new HiddenWait(); - private Constructor cachedEventConfigurationConstructor; private boolean staleMetadata = true; private boolean unregistered; private long lastUnloaded = -1; @@ -103,7 +103,7 @@ public final class MetadataRepository { List eventTypes = new ArrayList<>(configurations.size() + nativeEventTypes.size()); for (EventConfiguration ec : configurations) { if (ec.isRegistered()) { - eventTypes.add(ec.getEventType()); + eventTypes.add(ec.eventType()); } } for (EventType t : nativeEventTypes.values()) { @@ -117,7 +117,7 @@ public final class MetadataRepository { public synchronized EventType getEventType(Class eventClass) { EventConfiguration ec = getConfiguration(eventClass, false); if (ec != null && ec.isRegistered()) { - return ec.getEventType(); + return ec.eventType(); } throw new IllegalStateException("Event class " + eventClass.getName() + " is not registered"); } @@ -125,7 +125,7 @@ public final class MetadataRepository { public synchronized void unregister(Class eventClass) { EventConfiguration configuration = getConfiguration(eventClass, false); if (configuration != null) { - configuration.getPlatformEventType().setRegistered(false); + configuration.platformEventType().setRegistered(false); } // never registered, ignore call } @@ -147,14 +147,14 @@ public final class MetadataRepository { PlatformEventType pe = findMirrorType(eventClass); configuration = makeConfiguration(eventClass, pe, dynamicAnnotations, dynamicFields); } - configuration.getPlatformEventType().setRegistered(true); - TypeLibrary.addType(configuration.getPlatformEventType()); + configuration.platformEventType().setRegistered(true); + TypeLibrary.addType(configuration.platformEventType()); if (JVM.isRecording()) { - settingsManager.setEventControl(configuration.getEventControl(), true, JVM.counterTime()); + settingsManager.setEventControl(configuration.eventControl(), true, JVM.counterTime()); settingsManager.updateRetransform(Collections.singletonList((eventClass))); } setStaleMetadata(); - return configuration.getEventType(); + return configuration.eventType(); } private PlatformEventType findMirrorType(Class eventClass) throws InternalError { @@ -179,20 +179,6 @@ public final class MetadataRepository { return JVMSupport.getConfiguration(eventClass); } - private EventConfiguration newEventConfiguration(EventType eventType, EventControl ec) { - try { - if (cachedEventConfigurationConstructor == null) { - var argClasses = new Class[] { EventType.class, EventControl.class}; - Constructor c = EventConfiguration.class.getDeclaredConstructor(argClasses); - c.setAccessible(true); - cachedEventConfigurationConstructor = c; - } - return cachedEventConfigurationConstructor.newInstance(eventType, ec); - } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { - throw new InternalError(e); - } - } - private EventConfiguration makeConfiguration(Class eventClass, PlatformEventType pEventType, List dynamicAnnotations, List dynamicFields) throws InternalError { SecuritySupport.addInternalEventExport(eventClass); if (pEventType == null) { @@ -226,12 +212,12 @@ public final class MetadataRepository { } EventType eventType = PrivateAccess.getInstance().newEventType(pEventType); EventControl ec = new EventControl(pEventType, eventClass); - EventConfiguration configuration = newEventConfiguration(eventType, ec); - PlatformEventType pe = configuration.getPlatformEventType(); - pe.setRegistered(true); + SettingControl[] settings = ec.getSettingControls().toArray(new SettingControl[0]); + EventConfiguration configuration = new EventConfiguration(pEventType, eventType, ec, settings, eventType.getId()); + pEventType.setRegistered(true); // If class is instrumented or should not be instrumented, mark as instrumented. - if (JVM.isInstrumented(eventClass) || !JVMSupport.shouldInstrument(pe.isJDK(), pe.getName())) { - pe.setInstrumented(); + if (JVM.isInstrumented(eventClass) || !JVMSupport.shouldInstrument(pEventType.isJDK(), pEventType.getName())) { + pEventType.setInstrumented(); } JVMSupport.setConfiguration(eventClass, configuration); return configuration; @@ -254,7 +240,7 @@ public final class MetadataRepository { for (Class clazz : eventClasses) { EventConfiguration eh = JVMSupport.getConfiguration(clazz); if (eh != null) { - controls.add(eh.getEventControl()); + controls.add(eh.eventControl()); } } return controls; diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java index 4672220ba65..8521cd4303f 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -158,7 +158,7 @@ final class SettingsManager { for(Class eventClass: eventClasses) { EventConfiguration ec = JVMSupport.getConfiguration(eventClass); if (ec != null ) { - PlatformEventType eventType = ec.getPlatformEventType(); + PlatformEventType eventType = ec.platformEventType(); if (eventType.isMarkedForInstrumentation()) { classes.add(eventClass); eventType.markForInstrumentation(false); diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java index 04df4bb9954..4b0f2fba3da 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,38 +26,17 @@ package jdk.jfr.internal.event; import jdk.jfr.EventType; +import jdk.jfr.SettingControl; import jdk.jfr.internal.EventControl; import jdk.jfr.internal.JVM; import jdk.jfr.internal.PlatformEventType; -import jdk.jfr.internal.PrivateAccess; -import jdk.jfr.SettingControl; -// Users should not be able to subclass or instantiate for security reasons. -public final class EventConfiguration { - private final PlatformEventType platformEventType; - private final EventType eventType; - private final EventControl eventControl; - private final SettingControl[] settings; - - // Private constructor so user code can't instantiate - private EventConfiguration(EventType eventType, EventControl eventControl) { - this.eventType = eventType; - this.platformEventType = PrivateAccess.getInstance().getPlatformEventType(eventType); - this.eventControl = eventControl; - this.settings = eventControl.getSettingControls().toArray(new SettingControl[0]); - } - - // Class jdk.jfr.internal.PlatformEventType is not - // accessible from event class by design - public PlatformEventType getPlatformEventType() { - return platformEventType; - } - - // Class jdk.jfr.internal.EventControl is not - // accessible from event class by design - public EventControl getEventControl() { - return eventControl; - } +public record EventConfiguration( + PlatformEventType platformEventType, + EventType eventType, + EventControl eventControl, + SettingControl[] settings, + long id) { // Accessed by generated code in event class public boolean shouldCommit(long duration) { @@ -74,11 +53,6 @@ public final class EventConfiguration { return platformEventType.isCommittable(); } - // Accessed by generated code in event class - public EventType getEventType() { - return eventType; - } - // Not really part of the configuration, but the method // needs to be somewhere the event class can access, // but not part of the public API. @@ -100,8 +74,4 @@ public final class EventConfiguration { public boolean isRegistered() { return platformEventType.isRegistered(); } - - public long getId() { - return eventType.getId(); - } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java index 719fa4e1b4d..8dbac99067a 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java @@ -234,14 +234,14 @@ public final class EventWriter { public boolean beginEvent(EventConfiguration configuration, long typeId) { // This check makes sure the event type matches what was added by instrumentation. - if (configuration.getId() != typeId) { + if (configuration.id() != typeId) { throw new InternalError("Unexpected type id " + typeId); } if (excluded) { // thread is excluded from writing events return false; } - this.eventType = configuration.getPlatformEventType(); + this.eventType = configuration.platformEventType(); reserveEventSizeField(); putLong(eventType.getId()); return true;