Hi Bernd,

On 10/31/18 9:39 PM, Bernd Eckenfels wrote:
http://cr.openjdk.java.net/~dlong/8212605/webrev.1/src/java.base/share/classes/java/security/AccessController.java.udiff.html

In checkContext should the security manager be null checked first instead of 
last to optimize for the typical case? (If the side effects in that expression 
are desired it should be documented)

I was following the example of createWrapper.  The side-effects of getInnocuousAcc() will only happen once, so the order shouldn't matter here, except for performance reasons.  I don't have a strong opinion about the order, but it looks like the typical case for createWrapper would also be the typical case for checkContext, so maybe they both should be changed, if indeed a null security manager is the more typical case.

I find the tail call optimization comment in wrapException adds only confusion 
to an otherwise clear helper. But maybe it’s just me who does not understand it.

OK, I'm happy to remove it if it's confusing or not helpful.  The reason for the other @ForceInline and @ReservedStackAccess annotations is to reduce that chance of getting a StackOverflowError, and tail call elimination would also help with that.  Let's see if anyone else finds the comment helpful or confusing.

dl

Gruss
Bernd
--
http://bernd.eckenfels.net

________________________________
Von: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> im Auftrag von Vladimir 
Ivanov <vladimir.x.iva...@oracle.com>
Gesendet: Donnerstag, November 1, 2018 5:11 AM
An: dean.l...@oracle.com; security-dev@openjdk.java.net; core-libs-dev Libs; 
hotspot-dev developers
Betreff: Re: RFR(M) 8212605: Pure-Java implementation of 
AccessController.doPrivileged

Dean,

src/java.base/share/classes/java/security/AccessController.java:
+ /**
+ * Internal marker for hidden implementation frames.
+ */
+ /*non-public*/
+ @Target(ElementType.METHOD)
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface Hidden {
+ }

You declare @Hidden, but then map it to _method_Hidden along with
@Hidden from java.lang.invoke.LambdaForm.

What do you think about moving LambdaForm.Hidden to
jdk.internal.vm.annotation instead and share among all usages?

Best regards,
Vladimir Ivanov

On 31/10/2018 15:23, dean.l...@oracle.com wrote:
https://bugs.openjdk.java.net/browse/JDK-8212605
http://cr.openjdk.java.net/~dlong/8212605/webrev.1

This change implements AccessController.doPrivileged in Java.  This
gives a performance improvement while also being useful to Project Loom
by removing the Java --> native --> Java transition.  One reason
doPrivileged has historically been in native is because of the need to
guarantee the cleanup of the privileged context when doPrivileged
returns.  To do that in Java, the information that
AccessController.getContext needs is pushed onto the Java stack as
arguments to a method that getContext will recognize during its stack
walk.  This allows us to remove the native privileged stack while
guaranteeing that the privileged context goes away when the method returns.

Tested with tier1-tier3 hotspot and jdk tests and JCK api/java_security
tests.  For the first few rounds of testing, I kept the old native
privileged stack and compared the results of the old and new
implementations for each getContext call, which did catch some early
bugs.  The changes were also examined by internal security experts and
run through additional internal security tests.

The improvement on this [1] doPrivileged microbenchmark is approximate 50x.

There is no attempt to optimize getContext() or security permission
checks in this change, however, this is intended to be a first step
towards other possible improvements, for example those proposed here [2].

dl

[1]
http://hg.openjdk.java.net/code-tools/jmh-jdk-microbenchmarks/file/fc4783360f58/src/main/java/org/openjdk/bench/java/security/DoPrivileged.java

[2]
http://mail.openjdk.java.net/pipermail/security-dev/2017-December/016627.html



Reply via email to