Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Commit

Permalink
8268398: 15% increase in JFR footprint in Noop-Base
Browse files Browse the repository at this point in the history
Reviewed-by: jbachorik
  • Loading branch information
egahlin committed Jun 17, 2022
1 parent 983f75c commit 97544be
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 15 deletions.
12 changes: 12 additions & 0 deletions src/hotspot/share/jfr/jni/jfrJniMethod.cpp
Expand Up @@ -61,6 +61,10 @@
#include "runtime/thread.hpp"
#include "utilities/debug.hpp"

#ifdef LINUX
#include "osContainer_linux.hpp"
#endif

#define NO_TRANSITION(result_type, header) extern "C" { result_type JNICALL header {
#define NO_TRANSITION_END } }

Expand Down Expand Up @@ -381,3 +385,11 @@ JVM_END
JVM_ENTRY_NO_ENV(jboolean, jfr_is_class_instrumented(JNIEnv* env, jobject jvm, jclass clazz))
return JfrJavaSupport::is_instrumented(clazz, thread);
JVM_END

JVM_ENTRY_NO_ENV(jboolean, jfr_is_containerized(JNIEnv* env, jobject jvm))
#ifdef LINUX
return OSContainer::is_containerized();
#else
return false;
#endif
JVM_END
3 changes: 2 additions & 1 deletion src/hotspot/share/jfr/jni/jfrJniMethod.hpp
Expand Up @@ -158,9 +158,10 @@ jboolean JNICALL jfr_is_class_excluded(JNIEnv* env, jobject jvm, jclass clazz);

jboolean JNICALL jfr_is_class_instrumented(JNIEnv* env, jobject jvm, jclass clazz);

jboolean JNICALL jfr_is_containerized(JNIEnv* env, jobject jvm);

#ifdef __cplusplus
}
#endif

#endif // SHARE_JFR_JNI_JFRJNIMETHOD_HPP

3 changes: 2 additions & 1 deletion src/hotspot/share/jfr/jni/jfrJniMethodRegistration.cpp
Expand Up @@ -93,7 +93,8 @@ JfrJniMethodRegistration::JfrJniMethodRegistration(JNIEnv* env) {
(char*)"setConfiguration", (char*)"(Ljava/lang/Class;Ljdk/jfr/internal/event/EventConfiguration;)Z", (void*)jfr_set_configuration,
(char*)"getTypeId", (char*)"(Ljava/lang/String;)J", (void*)jfr_get_type_id_from_string,
(char*)"isExcluded", (char*)"(Ljava/lang/Class;)Z", (void*)jfr_is_class_excluded,
(char*)"isInstrumented", (char*)"(Ljava/lang/Class;)Z", (void*) jfr_is_class_instrumented
(char*)"isInstrumented", (char*)"(Ljava/lang/Class;)Z", (void*) jfr_is_class_instrumented,
(char*)"isContainerized", (char*)"()Z", (void*) jfr_is_containerized
};

const size_t method_array_length = sizeof(method) / sizeof(JNINativeMethod);
Expand Down
9 changes: 9 additions & 0 deletions src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java
Expand Up @@ -620,4 +620,13 @@ public boolean hasNativeJFR() {
* @return the id, or a negative value if it does not exists.
*/
public native long getTypeId(String name);

/**
* Returns {@code true}, if the JVM is running in a container, {@code false} otherwise.
* <p>
* If -XX:-UseContainerSupport has been specified, this method returns {@code false},
* which is questionable, but Container.metrics() returns {@code null}, so events
* can't be emitted anyway.
*/
public native boolean isContainerized();
}
8 changes: 8 additions & 0 deletions src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java
Expand Up @@ -59,6 +59,10 @@ final class JVMUpcalls {
static byte[] onRetransform(long traceId, boolean dummy1, boolean dummy2, Class<?> clazz, byte[] oldBytes) throws Throwable {
try {
if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && !Modifier.isAbstract(clazz.getModifiers())) {
if (!Utils.shouldInstrument(clazz.getClassLoader() == null, clazz.getName())) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Skipping instrumentation for " + clazz.getName() + " since container support is missing");
return oldBytes;
}
EventWriterKey.ensureEventWriterFactory();
EventConfiguration configuration = Utils.getConfiguration(clazz.asSubclass(jdk.internal.event.Event.class));
if (configuration == null) {
Expand Down Expand Up @@ -103,6 +107,10 @@ static byte[] bytesForEagerInstrumentation(long traceId, boolean forceInstrument
try {
EventInstrumentation ei = new EventInstrumentation(superClass, oldBytes, traceId, bootClassLoader, true);
eventName = ei.getEventName();
if (!Utils.shouldInstrument(bootClassLoader, ei.getEventName())) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Skipping instrumentation for " + eventName + " since container support is missing");
return oldBytes;
}
if (!forceInstrumentation) {
// Assume we are recording
MetadataRepository mr = MetadataRepository.getInstance();
Expand Down
Expand Up @@ -218,7 +218,8 @@ private EventConfiguration makeConfiguration(Class<? extends jdk.internal.event.
EventConfiguration configuration = newEventConfiguration(eventType, ec, settings);
PlatformEventType pe = configuration.getPlatformEventType();
pe.setRegistered(true);
if (jvm.isInstrumented(eventClass)) {
// If class is instrumented or should not be instrumented, mark as instrumented.
if (jvm.isInstrumented(eventClass) || !Utils.shouldInstrument(pe.isJDK(), pe.getName())) {
pe.setInstrumented();
}
Utils.setConfiguration(eventClass, configuration);
Expand Down
18 changes: 7 additions & 11 deletions src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java
Expand Up @@ -94,7 +94,6 @@ public final class Utils {
* This field will be lazily initialized and the access is not synchronized.
* The possible data race is benign and is worth of not introducing any contention here.
*/
private static Metrics[] metrics;
private static Instant lastTimestamp;

public static void checkAccessFlightRecorder() throws SecurityException {
Expand Down Expand Up @@ -698,18 +697,15 @@ public static String formatDuration(Duration d) {
}
}

public static boolean shouldSkipBytecode(String eventName, Class<?> superClass) {
if (superClass.getClassLoader() != null || !superClass.getName().equals("jdk.jfr.events.AbstractJDKEvent")) {
return false;
public static boolean shouldInstrument(boolean isJDK, String name) {
if (!isJDK) {
return true;
}
return eventName.startsWith("jdk.Container") && getMetrics() == null;
}

private static Metrics getMetrics() {
if (metrics == null) {
metrics = new Metrics[]{Metrics.systemMetrics()};
if (!name.contains(".Container")) {
// Didn't match @Name("jdk.jfr.Container*") or class name "jdk.jfr.events.Container*"
return true;
}
return metrics[0];
return JVM.getJVM().isContainerized();
}

private static String formatPositiveDuration(Duration d){
Expand Down
Expand Up @@ -174,7 +174,15 @@ public static void addInstrumentation() {
}

private static void initializeContainerEvents() {
containerMetrics = Container.metrics();
if (JVM.getJVM().isContainerized() ) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, "JVM is containerized");
containerMetrics = Container.metrics();
if (containerMetrics != null) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, "Container metrics are available");
}
}
// The registration of events and hooks are needed to provide metadata,
// even when not running in a container
SecuritySupport.registerEvent(ContainerConfigurationEvent.class);
SecuritySupport.registerEvent(ContainerCPUUsageEvent.class);
SecuritySupport.registerEvent(ContainerCPUThrottlingEvent.class);
Expand Down

1 comment on commit 97544be

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.