On Thu, 29 Sep 2022 15:12:06 GMT, Sean Coffey <coff...@openjdk.org> wrote:
>> With event streaming, beginChunk is usually to prefer. Otherwise, a client >> that monitors the JVM must wait until the first chunk rotation to get the >> data. >> >> That said, we want startup to be quick. There should probably be a common >> parameter, i.e. security=off/normal/audit/trace, that handles enablement for >> all security events. I don't know how expensive this event is and where it >> would fit among those categories? >> >> If the event triggers class loading, it might make sense to check if the >> event is enabled first. > > Thanks @egahlin - maybe we can leave it at beginChunk setting then. > > I've been doing some testing to satisfy myself that the impact of this event > on performance is minimal, Running the new `emitInitialSecurityProperties()` > is showing a cost of ~ 1.6ms (1602998 ns). > > This new Event itself doesn't trigger extra class loading AFAICT. I went back > to a jdk 20 binary without this patch and ran some tests. > > `ProtectionDomain ` is a very early class to initialize [1] (initPhase2) > > Without JFR, `java.security.Security` will initialize in a default JDK with a > JMX `Agent.startLocalManagementAgent` call in a simple HelloWorld test which > prints "Hello" and then sleeps [2] - the JMX thread starts after about 3 > seconds of runtime. > > Without JFR and by using the `-XX:+DisableAttachMechanism` option, the > Security class will not load in same test. > > If JFR is on, then Security class is already being loaded, even without this > patch [3] > > [1] > > at > java.base/java.security.ProtectionDomain.<clinit>(ProtectionDomain.java:64) > at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:316) > at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:431) > at > java.base/java.security.SecureClassLoader.<init>(SecureClassLoader.java:113) > at > java.base/jdk.internal.loader.BuiltinClassLoader.<init>(BuiltinClassLoader.java:194) > at > java.base/jdk.internal.loader.ClassLoaders$BootClassLoader.<init>(ClassLoaders.java:135) > at > java.base/jdk.internal.loader.ClassLoaders.<clinit>(ClassLoaders.java:79) > at > java.base/jdk.internal.loader.BootLoader.loadModule(BootLoader.java:120) > at > java.base/jdk.internal.module.ModuleBootstrap.boot2(ModuleBootstrap.java:266) > at > java.base/jdk.internal.module.ModuleBootstrap.boot(ModuleBootstrap.java:174) > at java.base/java.lang.System.initPhase2(System.java:2214) > > > [2] > > at java.base/java.security.Security.<clinit>(Security.java:73) > at > java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:93) > at > java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:90) > at > java.base/java.security.AccessController.doPrivileged(AccessController.java:319) > at > java.base/sun.net.InetAddressCachePolicy.<clinit>(InetAddressCachePolicy.java:89) > at > java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1005) > at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1658) > at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1524) > at java.base/java.net.InetAddress.getByName(InetAddress.java:1413) > at > jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startLocalConnectorServer(ConnectorBootstrap.java:531) > at > jdk.management.agent/jdk.internal.agent.Agent.startLocalManagementAgent(Agent.java:317) > > > [3] > > > at java.base/java.security.Security.<clinit>(Security.java:73) > at > java.base/sun.security.util.SecurityProperties.getOverridableProperty(SecurityProperties.java:57) > at > java.base/sun.security.util.SecurityProperties.privilegedGetOverridable(SecurityProperties.java:48) > at > java.base/sun.security.util.SecurityProperties.includedInExceptions(SecurityProperties.java:72) > at > java.base/sun.security.util.SecurityProperties.<clinit>(SecurityProperties.java:36) > at > java.base/sun.security.util.FilePermCompat.<clinit>(FilePermCompat.java:43) > at > java.base/java.security.AccessControlContext.<init>(AccessControlContext.java:270) > at > java.base/java.security.AccessController.createWrapper(AccessController.java:649) > at > java.base/java.security.AccessController.doPrivileged(AccessController.java:461) > at > jdk.jfr/jdk.jfr.internal.SecuritySupport.doPrivilegedWithReturn(SecuritySupport.java:261) > at > jdk.jfr/jdk.jfr.internal.SecuritySupport.getPathInProperty(SecuritySupport.java:331) > at > jdk.jfr/jdk.jfr.internal.SecuritySupport.<clinit>(SecuritySupport.java:80) > at > jdk.jfr/jdk.jfr.internal.JVMSupport.checkAvailability(JVMSupport.java:46) > at jdk.jfr/jdk.jfr.internal.JVMSupport.<clinit>(JVMSupport.java:41) > at jdk.jfr/jdk.jfr.internal.Logger.<clinit>(Logger.java:41) > at > jdk.jfr/jdk.jfr.internal.dcmd.AbstractDCmd.execute(AbstractDCmd.java:75) When support for the SM is removed, the dependency on `AccessController.doPrivileged` will be removed and there may no longer be a JFR dependency on loading the `Security` class. So, it is ok for now, but a solution that doesn't depend on this might be better in the long run. ------------- PR: https://git.openjdk.org/jdk/pull/10394
