Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread dean . long
Thanks Roger. dl On 11/5/18 2:00 PM, Roger Riggs wrote: Hi Dean, Looks ok, I have no better suggestion. Roger On 11/05/2018 01:51 PM, dean.l...@oracle.com wrote: Hi Roger.  Thanks for looking at this. On 11/5/18 7:21 AM, Roger Riggs wrote: Hi Dean, typo AccessController line788:

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread Roger Riggs
Hi Dean, Looks ok, I have no better suggestion. Roger On 11/05/2018 01:51 PM, dean.l...@oracle.com wrote: Hi Roger.  Thanks for looking at this. On 11/5/18 7:21 AM, Roger Riggs wrote: Hi Dean, typo AccessController line788: "annocations" Fixed. The implementations of

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread dean . long
Thanks Alan. dl On 11/4/18 1:03 AM, Alan Bateman wrote: On 03/11/2018 20:00, dean.l...@oracle.com wrote: I made a pass at improving the comments based on feedback I've received.  I updated webrev.4 in place, along with an incremental diff: I looked through the updated webrev.4, mostly

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread Roger Riggs
Hi Dean, typo AccessController line788: "annocations" The implementations of doPrivileged(PrivilegedExceptionAction action) and doPrivileged(PrivilegedAction action) Could be a bit more similar since except for the exception wrapping they are the same. 309 return executePrivileged(action,

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-04 Thread Alan Bateman
On 03/11/2018 20:00, dean.l...@oracle.com wrote: I made a pass at improving the comments based on feedback I've received.  I updated webrev.4 in place, along with an incremental diff: I looked through the updated webrev.4, mostly studying the changes in AccessController.java and jvm.cpp and the

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-03 Thread dean . long
I made a pass at improving the comments based on feedback I've received.  I updated webrev.4 in place, along with an incremental diff: http://cr.openjdk.java.net/~dlong/8212605/webrev.4.update/ dl On 10/31/18 9:39 PM, Bernd Eckenfels wrote: I find the tail call optimization comment in

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-02 Thread dean . long
Thanks Mandy.  I also appreciate you noticing (off-list) that I can remove the extra space in "Class " in several places.  I have updated webrev.4 in place. dl On 11/2/18 1:55 PM, Mandy Chung wrote: Hi Dean, I reviewed webrev.4 version.  It looks good.  Happy to see moving the doPrivileged

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-02 Thread Mandy Chung
Hi Dean, I reviewed webrev.4 version.  It looks good.  Happy to see moving the doPrivileged support to Java and the performance improvement. On 10/31/18 3:23 PM, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8212605 http://cr.openjdk.java.net/~dlong/8212605/webrev.1

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread dean . long
On 11/1/18 12:39 PM, Sean Mullan wrote: I also replaced getCallerPD with the faster getProtectionDomain and removed a stale comment about impliesCreateAccessControlContext being called by the VM. It should be safe to remove now, but I left it in to minimize changes. I would just remove it,

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Mandy Chung
On 11/1/18 2:34 PM, dean.l...@oracle.com wrote: @Hidden is internal and no CSR is needed. FYI.  JDK-8212620 is the RFE to consider providing a standard mechanism to hide frames from stack trace. OK, I already filed JDK-8213234 but I think I should just close it as a duplicate of

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread dean . long
On 11/1/18 1:45 PM, Mandy Chung wrote: On 11/1/18 1:18 AM, Vladimir Ivanov wrote: I think it's a good idea, but I believe it would require a CSR request. Do you mind if I file a separate issue for jdk.internal.vm.annotation.Hidden? Sure. Most of the annotations in

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Mandy Chung
On 11/1/18 1:18 AM, Vladimir Ivanov wrote: I think it's a good idea, but I believe it would require a CSR request. Do you mind if I file a separate issue for jdk.internal.vm.annotation.Hidden? Sure. Most of the annotations in jdk.internal.vm.annotation were originally located in

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Sean Mullan
On 11/1/18 3:21 PM, dean.l...@oracle.com wrote: On 11/1/18 9:48 AM, Sean Mullan wrote: On 11/1/18 1:29 AM, dean.l...@oracle.com wrote: On 10/31/18 9:39 PM, Bernd Eckenfels wrote:

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread dean . long
On 11/1/18 9:48 AM, Sean Mullan wrote: On 11/1/18 1:29 AM, dean.l...@oracle.com wrote: 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

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread dean . long
On 11/1/18 10:01 AM, Sean Mullan wrote: Some of the copyrights need to be updated to 2018. Fixed. All else looks good to me as I had reviewed an earlier version of this before. We have talked about doing this for a while now, so I am finally glad we and are able to pretty much eliminate

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Ioi Lam
On 10/31/18 5:13 PM, dean.l...@oracle.com wrote: On 10/31/18 4:06 PM, David Holmes wrote: Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have a few related comments:

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Vladimir Ivanov
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,

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Vladimir Ivanov
I think it's a good idea, but I believe it would require a CSR request. Do you mind if I file a separate issue for jdk.internal.vm.annotation.Hidden? Sure. Most of the annotations in jdk.internal.vm.annotation were originally located in java.lang.invoke. Not sure CSR will be required in

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Sean Mullan
On 10/31/18 9:15 PM, Peter wrote: Hello Dean & David, Interesting reading.   Is DomainCombiner still being considered for deprecation? There are no immediate plans to deprecate DomainCombiner, but we are looking at the SecurityManager (SM) and all of its related APIs and evaluating

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread dean . long
Hi Peter.  Thanks for the input.  I don't know about DomainCombiner, but perhaps someone else on this list does. dl On 10/31/18 6:15 PM, Peter wrote: Hello Dean & David, Interesting reading.   Is DomainCombiner still being considered for deprecation? Our code makes heavy use of

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Sean Mullan
Some of the copyrights need to be updated to 2018. All else looks good to me as I had reviewed an earlier version of this before. We have talked about doing this for a while now, so I am finally glad we and are able to pretty much eliminate one of the more common SecurityManager related

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-01 Thread Sean Mullan
On 11/1/18 1:29 AM, dean.l...@oracle.com wrote: 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

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread dean . long
...@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

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread Bernd Eckenfels
Vladimir Ivanov 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

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread dean . long
I think it's a good idea, but I believe it would require a CSR request.  Do you mind if I file a separate issue for jdk.internal.vm.annotation.Hidden? dl On 10/31/18 6:11 PM, Vladimir Ivanov wrote: Dean, src/java.base/share/classes/java/security/AccessController.java: +    /** + *

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread dean . long
Thanks Ioi. dl On 10/31/18 6:01 PM, Ioi Lam wrote: On 10/31/18 5:13 PM, dean.l...@oracle.com wrote: On 10/31/18 4:06 PM, David Holmes wrote: Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have a few related

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread dean . long
Thanks David. dl On 10/31/18 5:54 PM, David Holmes wrote: Hi Dean, On 1/11/2018 10:13 AM, dean.l...@oracle.com wrote: On 10/31/18 4:06 PM, David Holmes wrote: Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread Peter
Hello Dean & David, Interesting reading. Is DomainCombiner still being considered for deprecation? Our code makes heavy use of AccessController.doPrivileged. Regards, Peter. "Hot Spots - Method","Self time [%]","Self time","Self time (CPU)","Samples"

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread David Holmes
Hi Dean, On 1/11/2018 10:13 AM, dean.l...@oracle.com wrote: On 10/31/18 4:06 PM, David Holmes wrote: Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have a few related comments:

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread dean . long
On 10/31/18 4:06 PM, David Holmes wrote: Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have a few related comments: src/hotspot/share/classfile/systemDictionary.hpp You added the java_security_AccessController

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-10-31 Thread David Holmes
Hi Dean, Looking only at the hotspot changes. The removal of the DoPrivileged and related privileged_stack code seems okay. I have a few related comments: src/hotspot/share/classfile/systemDictionary.hpp You added the java_security_AccessController class after