On Fri, 30 Sep 2022 19:32:29 GMT, Sean Mullan <mul...@openjdk.org> wrote:

>> 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.

fair point. Worse case scenario is that we don't record security properties on 
start up. Not the case for now though. I've added some extra code to check for 
this in the test.

-------------

PR: https://git.openjdk.org/jdk/pull/10394

Reply via email to