Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

2020-11-05 Thread Dean Long
On Thu, 5 Nov 2020 15:47:31 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long f

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

2020-11-05 Thread Dean Long
On Thu, 5 Nov 2020 23:58:21 GMT, Dean Long wrote: >> Roland Westrelin has updated the pull request with a new target base due to >> a merge or a rebase. The pull request now contains ten commits: >> >> - Jorn's comments >> - Update headers and add

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v7]

2020-11-17 Thread Dean Long
On Tue, 17 Nov 2020 08:33:20 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Dean Long
I looked at the AOT, C2, and JVMCI changes and I didn't find any issues. dl On 3/26/20 4:57 PM, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area. Small changes are made in javac, VM compiler (intrinsifica

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

2021-04-09 Thread Dean Long
On Fri, 9 Apr 2021 16:54:51 GMT, Ioi Lam wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > LGTM. Just a small nit. @iklam I thought the fingerprint code was also us

Re: [jdk18] RFR: 8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation

2021-12-15 Thread Dean Long
On Wed, 15 Dec 2021 05:59:45 GMT, Vladimir Kozlov wrote: > A proper fix for this is to use the catchException combination. However, that > introduces significant cold startup performance regression. JDK-8278447 > tracks the work to address the performance regression using catchException > and

Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination

2021-12-17 Thread Dean Long
On Mon, 13 Dec 2021 09:39:55 GMT, Сергей Цыпанов wrote: > Originally this was spotted by by Amir Hadadi in > https://stackoverflow.com/questions/70272651/missing-bounds-checking-elimination-in-string-constructor > > It looks like in the following code in `String(byte[], int, int, Charset)` > >

Re: [jdk18] RFR: 8281713: [BACKOUT] AArch64: Implement string_compare intrinsic in SVE

2022-02-14 Thread Dean Long
On Mon, 14 Feb 2022 10:30:09 GMT, Tobias Hartmann wrote: > We observed several failures on macosx aarch64 after integration of > [JDK-8275448](https://bugs.openjdk.java.net/browse/JDK-8275448). I could > reliably reproduce > [JDK-8281512](https://bugs.openjdk.java.net/browse/JDK-8281512) with

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Dean Long
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Dean Long
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

2015-02-13 Thread Dean Long
My understanding is that whether or not aarch64 allows unaligned accesses is based on a system setting, so this change is too simplistic. I would prefer that this was controlled with something more flexible, like "sun.cpu.unaligned". dl On 2/13/2015 5:38 AM, Andrew Haley wrote: java.​nio.​Di

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

2015-02-13 Thread Dean Long
On 2/13/2015 3:04 PM, chris...@zoulas.com wrote: On Feb 13, 2:52pm, dean.l...@oracle.com (Dean Long) wrote: -- Subject: Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer | My understanding is that whether or not aarch64 allows unaligned=20 | accesses is based on a | system

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

2015-02-13 Thread Dean Long
6 has flag UseUnalignedLoadStores which is set to true depending on which version of CPU VM runs. The CPU version is determined based on CPUID instruction results. Does AARCH64 has something similar? Regards, Vladimir On 2/13/15 3:41 PM, Dean Long wrote: On 2/13/2015 3:04 PM, chris...@zoulas

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

2015-02-14 Thread Dean Long
On 2/14/2015 12:07 AM, Andrew Haley wrote: On 02/13/2015 10:52 PM, Dean Long wrote: My understanding is that whether or not aarch64 allows unaligned accesses is based on a system setting, so this change is too simplistic. Disabling unaligned access would be a really perverse thing to do, and

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

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

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 c

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

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: +    /** + * Intern

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

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

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 one

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 security

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 jdk.internal.vm.annotat

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, so

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

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 studying

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

2018-11-05 Thread dean . long
Fixed.  Thanks. dl On 11/5/18 8:00 AM, Sean Mullan wrote: 726 * The VM recoginizes this method as special, so any changes to the s/recoginizes/recognizes/ --Sean On 11/3/18 4:00 PM, dean.l...@oracle.com wrote: I made a pass at improving the comments based on feedback I've received.  I updat

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-16 Thread dean . long
If we are adding JDK_EXPORT, does it make sense to add JDK_CALL as well? dl On 12/16/18 4:57 PM, David Holmes wrote: Hi Magnus, Thanks for explaining how addition of JNIEXPORT may have started this problem. One follow up: This will also need a CSR request due to the change in linking beha

Re: RFR: 8221723: Avoid storing zero to String.hash

2019-04-01 Thread dean . long
Wouldn't it be better to write a non-0 value when the computed hash code is 0, so we don't have to recompute it?  Is there some advantage to writing 0 instead of any other value, such as 1? dl On 4/1/19 4:57 AM, Claes Redestad wrote: Hi, when a String has a calculated hash code value of 0, w

Re: RFR: 8221723: Avoid storing zero to String.hash

2019-04-01 Thread dean . long
OK, I guess there's no ideal solution.  Adding a separate "not-computed" boolean adds space, and using a different sentinel value for "not-computed" would probably be slower on CPUs that have a fast compare-and-branch-against-zero instruction. dl On 4/1/19 12:55 PM, Martin Buchholz wrote: Th

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-16 Thread dean . long
On 3/15/17 6:19 PM, David Holmes wrote: src/share/vm/classfile/javaClasses.hpp Given the the only call to java_lang_String::set_debug_intrinsics is within an ifdef, shouldn't the declaration and definition of the method also be guarded the same way? OK I'll change it. dl

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-16 Thread dean . long
On 3/16/17 2:52 AM, Tobias Hartmann wrote: As a safety net, I added asserts around the intrinsic calls, and a try/catch that so any out of bounds exception turns into an assert error as well. So the assert and try/catch are only necessary to catch invalid offsets passed to the C1 intrinsic, ri

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-17 Thread dean . long
On 3/17/17 5:58 AM, Vladimir Ivanov wrote: I have the same concern. Can we fix the immediate problem in 9 and integrate verification logic in 10? OK, Tobias is suggesting having verification logic only inside the intrinsics. Are you suggesting removing that as well? Yes and put them back

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-17 Thread dean . long
I posted two new versions, webrev.1 keeping the Trusted inner class: http://cr.openjdk.java.net/~dlong/8158168/webrev.1/ and webrev.2 with it removed: http://cr.openjdk.java.net/~dlong/8158168/webrev.2/ dl On 3/17/17 5:58 AM, Vladimir Ivanov wrote: I have the same concern. Can we fix the i

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-21 Thread dean . long
On 3/20/17 11:10 PM, Xueming Shen wrote: Hi Dean, Thanks for doing this. Though as I suggested last time personally I prefer to make minimum change to simply seal those holes in ASB at this late stage of JDK9. I'm fine with the webrev.2 and it looks better and reasonable to pull all UTF16 o

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-21 Thread dean . long
On 3/21/17 9:37 AM, Vladimir Ivanov wrote: and webrev.2 with it removed: http://cr.openjdk.java.net/~dlong/8158168/webrev.2/ Thanks, Dean. I started with webrev.2 and tried to minimize the changes. I ended up with the following version: http://cr.openjdk.java.net/~vlivanov/dlong/8158168/

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-21 Thread dean . long
On 3/21/17 5:02 PM, David Holmes wrote: I haven't been looking at the details of this but have been watching from afar. As per my comments in the bug report (now public) I'm quite concerned about the thread-non-safety issue here ... On 22/03/2017 4:47 AM, dean.l...@oracle.com wrote: On 3/21/

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-22 Thread dean . long
Vladimir, don't you need to replace checkIndex with checkOffset in indexOf and lastIndexOf, so that we allow count == length? dl On 3/22/17 8:35 AM, Vladimir Ivanov wrote: So are we convinced that the proposed changes will never lead to a crash due to a missing or incorrect bounds check, due

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-22 Thread dean . long
Also, it looks like the changes I made to ASB.appendChars(char[] s, int off, int end) are not needed. dl On 3/22/17 11:01 AM, dean.l...@oracle.com wrote: Vladimir, don't you need to replace checkIndex with checkOffset in indexOf and lastIndexOf, so that we allow count == length? dl On 3/2

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-23 Thread dean . long
On 3/22/17 1:49 PM, Vladimir Ivanov wrote: Also, it looks like the changes I made to ASB.appendChars(char[] s, int off, int end) are not needed. Agree. Vladimir, don't you need to replace checkIndex with checkOffset in indexOf and lastIndexOf, so that we allow count == length? Yes, my bad.

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-23 Thread dean . long
On 3/23/17 11:25 AM, dean.l...@oracle.com wrote: On 3/22/17 1:49 PM, Vladimir Ivanov wrote: Also, it looks like the changes I made to ASB.appendChars(char[] s, int off, int end) are not needed. Agree. Vladimir, don't you need to replace checkIndex with checkOffset in indexOf and lastIndexO

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-30 Thread dean . long
I would like to go with the webrev.2 version, with asserts removed. All the reviewers have said they are OK with that version, and it allows more complete testing than the minimal version. dl On 3/23/17 12:03 PM, dean.l...@oracle.com wrote: On 3/23/17 11:25 AM, dean.l...@oracle.com wrote:

Re: [9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8

2017-03-30 Thread dean . long
Actually, the assert inside getChar/putChar shouldn't affect inlining, because C1 and C2 will use the intrinsic instead, so I'd like to use webrev.2 as is, so I don't have to re-run all the tests. dl On 3/30/17 11:24 AM, dean.l...@oracle.com wrote: I would like to go with the webrev.2 versi

Re: Request for review- RFE 8005716

2013-03-06 Thread Dean Long
On 3/5/2013 8:13 PM, BILL PITTORE wrote: On 3/5/2013 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like "_JNI_OnLoad@8" on Windows, you can't just turn that into "_JNI_OnLoad@8_" + . I think it needs to be "_JNI_OnLoad_" + + "@

Re: Request for review- RFE 8005716

2013-03-06 Thread Dean Long
If JNI_ONLOAD_SYMBOLS contains something like "_JNI_OnLoad@8" on Windows, you can't just turn that into "_JNI_OnLoad@8_" + . I think it needs to be "_JNI_OnLoad_" + + "@8" Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of add

Re: Request for review- RFE 8005716

2013-03-07 Thread Dean Long
hen the JNI_OnLoad_L function exported by the library Should these read "linked with the VM", or "linked with the native application loading the VM"? Jeremy On Wed, Mar 6, 2013 at 8:21 AM, Bob Vandette <mailto:bob.vande...@oracle.com>> wrote: On Ma

Re: RFC 7038914: VM could throw uncaught OOME in ReferenceHandler thread

2013-05-10 Thread Dean Long
If you really want to bullet-proof ReferenceHandler (and other system threads) against OOME caused by native allocations, then don't forget about monitor inflation if there is contention for "lock" :-) dl On 5/10/2013 6:14 AM, David Holmes wrote: Hi Peter, So it would appear that it is not i

Re: RFC 7038914: VM could throw uncaught OOME in ReferenceHandler thread

2013-05-10 Thread Dean Long
On 5/10/2013 1:22 PM, Peter Levart wrote: On 05/10/2013 10:08 PM, Peter Levart wrote: On 05/10/2013 08:10 PM, Dean Long wrote: If you really want to bullet-proof ReferenceHandler (and other system threads) against OOME caused by native allocations, then don't forget about monitor infl