RFR: JDK-8035930 - Check jdk/src/windows/native/java/io/io_util_md.c for JNI pending exceptions
Hi please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035930/webrev/ to address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035930 this adds additional NULL check for malloc and JNI calls and check for pending exceptions as appropriate regards Mark
Re: JEP 193: Enhanced Volatiles
Am 03.03.2014 20:26, schrieb mark.reinh...@oracle.com: Posted: http://openjdk.java.net/jeps/193 - Mark Reading about ideas to integrate atomic operations into the language, I was asking myself which of these operations I was using most frequently in my own code (currently by means of java.util.concurrent.atomic.AtomicXXX variables) and how they could be made more convenient with built-in support. Here's my top 5 list: 1. AtomicReference.compareAndSet(T expect,T update). In almost all of the usecases, expect was null. 2. AtomicBoolean.compareAndSet(boolean expect, boolean update. In almost all of the usecases, expect was false. 3. AtomicReference.getAndSet(T update). 4. AtomicInteger.getAndIncrement(). 5. AtomicInteger.getAndDecrement(). Now, 4. and 5. would be the easiest to solve: Make postfix ++ and -- implicitly atomic if used on volatile variables. I know that this would require a change of the JLS, but on the other hand, the precise meaning of volatile has been changed once before, so would this be a showstopper? For cases 1. and 2. one could think of a new operator lhs ?= rhs with the meaning Assign the rhs to the lhs only if lhs is currently null. Return true if the assignment took place (Alternatively, return rhs if the assignment took place. Would be more consistent with other assignment operators and convey the same amount of information as a boolean value). Make the operation atomic if lhs is a volatile variable. For case 3. one could devise an operator lhs := rhs with the meaning Assign the rhs to the lhs and return the previous content of the lhs. Make the operation atomic if lhs is a volatile variable. Note that the new operators would have consistent meaning even if used with non-volatile variables (even though they wouldn't provide exciting new possibilities)
Re: RFR: JDK-8035930 - Check jdk/src/windows/native/java/io/io_util_md.c for JNI pending exceptions
On 05/03/2014 11:28, Mark Sheppard wrote: Hi please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035930/webrev/ to address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035930 this adds additional NULL check for malloc and JNI calls and check for pending exceptions as appropriate Just two comments on this: 1. At line 213 then I think it would be better to return with the OOME pending rather than suppressing it and throwing a FNFE. In practical terms then if we are really out of native memory then it won't get very far but it would still be confusing to have FNF thrown for this case. 2. What you think about dropping pathToNTPath: from the exception messages? It's not clear to me that it helps and it also means these new exceptions are inconsistent with the other usages. -Alan.
Re: RFR: JDK-8035930 - Check jdk/src/windows/native/java/io/io_util_md.c for JNI pending exceptions
Hi Alan, thanks for the reply, I'll make the suggested amendments regards Mark On 05/03/2014 11:51, Alan Bateman wrote: On 05/03/2014 11:28, Mark Sheppard wrote: Hi please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035930/webrev/ to address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035930 this adds additional NULL check for malloc and JNI calls and check for pending exceptions as appropriate Just two comments on this: 1. At line 213 then I think it would be better to return with the OOME pending rather than suppressing it and throwing a FNFE. In practical terms then if we are really out of native memory then it won't get very far but it would still be confusing to have FNF thrown for this case. 2. What you think about dropping pathToNTPath: from the exception messages? It's not clear to me that it helps and it also means these new exceptions are inconsistent with the other usages. -Alan.
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
A few comments on catching up with this discussion... tryMonitorEnter() does no harm and it is out of reach of most programmers. Built-in monitors are heavily optimized for typical cases, which does not include any form of tryLock. So the hotspot implementation of tryMonitorEnter is very slow. Making it less slow would probably negatively impact typical cases. So tryMonitorEnter is only useful in cases where you absolutely need the functionality at any cost, and don't want to use a j.u.c Lock. It's not surprising that there almost no known usages, and probably none that justify continued support. The main downside of using a j.u.c Lock just for the sake of tryLock is that you cannot take advantage of the JVM's extremely compact lock representation (basically one bit in the object header) for never-locked objects. On the other hand, if you are trying to minimize footprint, there are usually ways to cope. For example, in a few places inside j.u.c. we've used other available bits as spinlocks and resorted to monitors/Locks only when necessary. In a sense, this emulates monitor inflation. -Doug
RE: JEP 193: Enhanced Volatiles
The JVM should be picking up the annotations (otherwise, you'd still need to design a public API to deal with atomics). BTW, it was explained to me at the JVM Language Summit that the commonly held belief about annotations, is not in fact true. Regards, Jeroen -Original Message- From: paulus.benedic...@gmail.com [mailto:paulus.benedic...@gmail.com] On Behalf Of Paul Benedict Sent: Wednesday, March 5, 2014 16:19 To: Christoph Engelbert Cc: Jeroen Frijters; core-libs-dev@openjdk.java.net Subject: Re: JEP 193: Enhanced Volatiles This idea really close very the commonly held belief that annotations shouldn't be used to extend the language -- and thus will not be accepted. If the compiler is picking up these annotations to emit different code, that is obviously shoehorning annotations into a language feature. On Wed, Mar 5, 2014 at 1:51 AM, Christoph Engelbert m...@noctarius.com mailto:m...@noctarius.com wrote: Am 05.03.2014 um 08:40 schrieb Jeroen Frijters jer...@sumatra.nl mailto:jer...@sumatra.nl : My goal here is to make sure that expert users can get their job done somehow, *without* making the job of mainstream developers harder. The add lvalues to Java so experts can write CAS-libraries fails that test miserably. Why not go for something far less intrusive then? Here's my straw man proposal: Add an annotation that can be placed on native methods to synthesize atomic accessor methods. Example usage: @AtomicField(next) private native boolean compareAndSet(Node expected, Node newValue); @AtomicArray private static native boolean compareAndSet(Node[] array, Node expected, Node newValue); (Note that the method name is not significant, the operation can be derived from the signature, or explicit in the annotation, if necessary.) This requires no changes to the language and adds only a slight burden on the developer (but it's very easy to add tooling support). Regards, Jeroen That looks like a good fit towards what was mentioned earlier using method handles. The JVM or compiler could just look out for those annotations and generate corresponding method / field handles to execute it. Thanks, Chris -- Cheers, Paul
Re: JEP 193: Enhanced Volatiles
Jeroen, I agree with you (1) not everyone holds the belief and (2) this is not a technical limitation. This is simply a matter of perspective and intent. However, those in charge in the JDK seem pretty strong on #1 -- at least from what I've seen in my interactions. That's why I don't think this will gain traction, but I am always willing to be surprised. On Wed, Mar 5, 2014 at 9:38 AM, Jeroen Frijters jer...@sumatra.nl wrote: The JVM should be picking up the annotations (otherwise, you'd still need to design a public API to deal with atomics). BTW, it was explained to me at the JVM Language Summit that the commonly held belief about annotations, is not in fact true. Regards, Jeroen -Original Message- From: paulus.benedic...@gmail.com [mailto:paulus.benedic...@gmail.com] On Behalf Of Paul Benedict Sent: Wednesday, March 5, 2014 16:19 To: Christoph Engelbert Cc: Jeroen Frijters; core-libs-dev@openjdk.java.net Subject: Re: JEP 193: Enhanced Volatiles This idea really close very the commonly held belief that annotations shouldn't be used to extend the language -- and thus will not be accepted. If the compiler is picking up these annotations to emit different code, that is obviously shoehorning annotations into a language feature. On Wed, Mar 5, 2014 at 1:51 AM, Christoph Engelbert m...@noctarius.com mailto:m...@noctarius.com wrote: Am 05.03.2014 um 08:40 schrieb Jeroen Frijters jer...@sumatra.nl mailto:jer...@sumatra.nl : My goal here is to make sure that expert users can get their job done somehow, *without* making the job of mainstream developers harder. The add lvalues to Java so experts can write CAS-libraries fails that test miserably. Why not go for something far less intrusive then? Here's my straw man proposal: Add an annotation that can be placed on native methods to synthesize atomic accessor methods. Example usage: @AtomicField(next) private native boolean compareAndSet(Node expected, Node newValue); @AtomicArray private static native boolean compareAndSet(Node[] array, Node expected, Node newValue); (Note that the method name is not significant, the operation can be derived from the signature, or explicit in the annotation, if necessary.) This requires no changes to the language and adds only a slight burden on the developer (but it's very easy to add tooling support). Regards, Jeroen That looks like a good fit towards what was mentioned earlier using method handles. The JVM or compiler could just look out for those annotations and generate corresponding method / field handles to execute it. Thanks, Chris -- Cheers, Paul -- Cheers, Paul
Re: JEP 193: Enhanced Volatiles
(This is already-traveled ground.) The ++/--/+= trick is cute as a shorthand, but without a means of expressing a full-blown CAS, falls short of the goal of integrate atomic read-modify-write operations into the programming model. IF the latter problem was addressed, we might consider redefining ++ and friends to be shorthand for an RMW operation, and extending that to be atomic if applied on volatiles. But only if we can get all the way there. The new operator addresses the above-mentioned hole (though will confuse the hell out of the 99.99% of users that will never use it, but who will still see it on the table of operators in their CS 101 textbook), but runs up against of a different goal; expressing not only atomic operations, but all the possible fencing modes. There's no way we're going to invent a new syntax for each fencing mode, and we don't even know that we will never add new fencing modes. One of the goals here is to align with JMM9, which has a goal of aligning with the recently minted C++ MM (so that we can have a coherent MM across Java and native code.) Though, if we were willing to separate fencing from access, this idea might re-emerge as viable. Reading about ideas to integrate atomic operations into the language, I was asking myself which of these operations I was using most frequently in my own code (currently by means of java.util.concurrent.atomic.AtomicXXX variables) and how they could be made more convenient with built-in support. Here's my top 5 list: 1. AtomicReference.compareAndSet(T expect,T update). In almost all of the usecases, expect was null. 2. AtomicBoolean.compareAndSet(boolean expect, boolean update. In almost all of the usecases, expect was false. 3. AtomicReference.getAndSet(T update). 4. AtomicInteger.getAndIncrement(). 5. AtomicInteger.getAndDecrement(). Now, 4. and 5. would be the easiest to solve: Make postfix ++ and -- implicitly atomic if used on volatile variables. I know that this would require a change of the JLS, but on the other hand, the precise meaning of volatile has been changed once before, so would this be a showstopper? For cases 1. and 2. one could think of a new operator lhs ?= rhs with the meaning Assign the rhs to the lhs only if lhs is currently null. Return true if the assignment took place (Alternatively, return rhs if the assignment took place. Would be more consistent with other assignment operators and convey the same amount of information as a boolean value). Make the operation atomic if lhs is a volatile variable. For case 3. one could devise an operator lhs := rhs with the meaning Assign the rhs to the lhs and return the previous content of the lhs. Make the operation atomic if lhs is a volatile variable. Note that the new operators would have consistent meaning even if used with non-volatile variables (even though they wouldn't provide exciting new possibilities)
Re: JEP 193: Enhanced Volatiles
Why not go for something far less intrusive then? I'm all for unintrusive. Though note that the intrusiveness metric on language features I(f) is not uniform across observers :) Here's my straw man proposal: Add an annotation that can be placed on native methods to synthesize atomic accessor methods. I suspect you were expecting this response: we don't add language semantics through annotations. I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others.
Re: JEP 193: Enhanced Volatiles
On 03/05/2014 05:55 PM, Jeroen Frijters wrote: Brian Goetz wrote: I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. And the JVM is the one implementing the language semantics (together with javac which feeds the JVM with bytecodes). Language semantcis are implemented by the combination of javac and JVM. If you say that this feature does not require any change to javac, you're just saying that you put all the burden on the JVM, but you *are* implementing the language semantics using annotations nevertheless... Regards, Peter I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen
Re: JEP 193: Enhanced Volatiles
On 03/05/2014 06:07 PM, Peter Levart wrote: On 03/05/2014 05:55 PM, Jeroen Frijters wrote: Brian Goetz wrote: I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. And the JVM is the one implementing the language semantics (together with javac which feeds the JVM with bytecodes). Language semantcis are implemented by the combination of javac and JVM. If you say that this feature does not require any change to javac, you're just saying that you put all the burden on the JVM, but you *are* implementing the language semantics using annotations nevertheless... no, it's the other way around. javac maps Java the language to the bytecode semantics. The JVM only execute bytecodes. Regards, Peter cheers, Rémi I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen
Re: JEP 193: Enhanced Volatiles
I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. BTW, as I mentioned in another post in this thread, I specifically asked about this at the JVM Language Summit (in 2012 IIRC) and the answer was (by Alex IIRC) that there is no such rule. Perhaps, but this isn't a court of law :) And the spirit here is pretty clear. But, keep trying! I know you're trying to help,and it only takes one good idea.
RE: JEP 193: Enhanced Volatiles
Brian Goetz wrote: I'm all for unintrusive. Though note that the intrusiveness metric on language features I(f) is not uniform across observers :) Indeed :-) Here's my straw man proposal: Add an annotation that can be placed on native methods to synthesize atomic accessor methods. I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. BTW, as I mentioned in another post in this thread, I specifically asked about this at the JVM Language Summit (in 2012 IIRC) and the answer was (by Alex IIRC) that there is no such rule. I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen
Re: JEP 193: Enhanced Volatiles
On 03/05/2014 05:55 PM, Jeroen Frijters wrote: Brian Goetz wrote: I'm all for unintrusive. Though note that the intrusiveness metric on language features I(f) is not uniform across observers :) Indeed :-) Here's my straw man proposal: Add an annotation that can be placed on native methods to synthesize atomic accessor methods. I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. BTW, as I mentioned in another post in this thread, I specifically asked about this at the JVM Language Summit (in 2012 IIRC) and the answer was (by Alex IIRC) that there is no such rule. and that's not true, the meta-annotations @Inherited or @Target change the semantics of the annotated annotation. I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen Rémi
Re: JDK 9 RFR to remove a raw lint warning from java/lang/invoke/MethodHandleImpl.java
*ping*
Fixing this issue will restore raw lint warning cleanliness to core libs :-)
Thanks,
-Joe
On 03/03/2014 06:03 PM, Joe Darcy wrote:
Hello,
The recent changeset for
8027827: Improve performance of catchException combinator
8034120: MethodHandles.catchException doesn't handle
VarargsCollector right
introduced a raw lint warning to the core libraries. Please review the
patch below which removes the warning by adding ? extends
Throwable to the type the exType parameter of the method. A JDK
build succeeds with the new signature in place.
Thanks,
-Joe
diff -r 6cfedc362f48
src/share/classes/java/lang/invoke/MethodHandleImpl.java
--- a/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon Mar
03 18:17:00 2014 +0400
+++ b/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon Mar
03 17:49:52 2014 -0800
@@ -734,7 +734,7 @@
* (see {@link InvokerBytecodeGenerator#emitGuardWithCatch
emitGuardWithCatch}).
*/
@LambdaForm.Hidden
-static Object guardWithCatch(MethodHandle target, Class exType,
MethodHandle catcher,
+static Object guardWithCatch(MethodHandle target, Class? extends
Throwable exType, MethodHandle catcher,
Object... av) throws Throwable {
try {
return target.invokeWithArguments(av);
Re: JDK 9 RFR to remove a raw lint warning from java/lang/invoke/MethodHandleImpl.java
On 03/05/2014 07:56 PM, Joe Darcy wrote:
*ping*
Fixing this issue will restore raw lint warning cleanliness to core
libs :-)
Thanks,
-Joe
looks good :)
Rémi
On 03/03/2014 06:03 PM, Joe Darcy wrote:
Hello,
The recent changeset for
8027827: Improve performance of catchException combinator
8034120: MethodHandles.catchException doesn't handle
VarargsCollector right
introduced a raw lint warning to the core libraries. Please review
the patch below which removes the warning by adding ? extends
Throwable to the type the exType parameter of the method. A JDK
build succeeds with the new signature in place.
Thanks,
-Joe
diff -r 6cfedc362f48
src/share/classes/java/lang/invoke/MethodHandleImpl.java
--- a/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon
Mar 03 18:17:00 2014 +0400
+++ b/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon
Mar 03 17:49:52 2014 -0800
@@ -734,7 +734,7 @@
* (see {@link InvokerBytecodeGenerator#emitGuardWithCatch
emitGuardWithCatch}).
*/
@LambdaForm.Hidden
-static Object guardWithCatch(MethodHandle target, Class exType,
MethodHandle catcher,
+static Object guardWithCatch(MethodHandle target, Class?
extends Throwable exType, MethodHandle catcher,
Object... av) throws Throwable {
try {
return target.invokeWithArguments(av);
Re: RFR: JDK-8035930 - Check jdk/src/windows/native/java/io/io_util_md.c for JNI pending exceptions
Hi webrev has been updated as per Alan's suggestions http://cr.openjdk.java.net/~msheppar/8035930/webrev.01/ regards Mark On 05/03/2014 11:51, Alan Bateman wrote: On 05/03/2014 11:28, Mark Sheppard wrote: Hi please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035930/webrev/ to address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035930 this adds additional NULL check for malloc and JNI calls and check for pending exceptions as appropriate Just two comments on this: 1. At line 213 then I think it would be better to return with the OOME pending rather than suppressing it and throwing a FNFE. In practical terms then if we are really out of native memory then it won't get very far but it would still be confusing to have FNF thrown for this case. 2. What you think about dropping pathToNTPath: from the exception messages? It's not clear to me that it helps and it also means these new exceptions are inconsistent with the other usages. -Alan.
Re: RFR: JDK-8035930 - Check jdk/src/windows/native/java/io/io_util_md.c for JNI pending exceptions
On 05/03/2014 19:47, Mark Sheppard wrote: Hi webrev has been updated as per Alan's suggestions http://cr.openjdk.java.net/~msheppar/8035930/webrev.01/ This looks to me. A minor nit is that there is a leading space in each exception message now. -Alan.
RFR: JDK-8035340 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
Hi Please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035340/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035340 This effort adds NULL checks and pending exception check after JNI calls and adds NULL check to malloc calls in WindowsPreferences.c This is a Windows change set. regards Mark
RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? (2) StringBuilder can be used to replace the StringBuffer in toString(). -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David
Re: JEP 193: Enhanced Volatiles
On 03/03/2014 04:25 PM, Brian Goetz wrote: Posted: http://openjdk.java.net/jeps/193 Some follow-up thoughts on teasing apart the issues covered by this JEP. There are three main layers of questions to answer: 1. How do we surface the various pieces of this into the programming model? This includes language syntax (e.g., x.volatile.compareAndSet()), library surface (e.g., the fictitious and not-terribly-satisfying VolatileInt interface), and relevant restrictions (e.g., can we do volatile operations on non-volatile fields or array elements?) Also, relatedly, how far is this taken? It is easy to imagine operations of this sort on reference fields and fields of all 8 primitive types, as well as arrays of all 8 primitive types, and object arrays (which would of course cover all arrays-of-arrays as well). Following the interface idea, you would need (at a maximum) one interface type for each primitive type and one interface type for Objects. Arrays should be able to reuse the same interface type since array element types are constrained to the same set of possible types as fields (though without an actual volatile qualifier of course). So 9 interface types at maximum. At minimum, just having interfaces for ints, longs, and Objects would match what is presently defined in j.u.c.atomic, so 3 seems to be the low end. Personally I think it would be nicest to support all 8, but I expect there were probably good reasons (I'm guessing relating to false sharing) that we don't have AtomicBooleanArrays and so forth. Other random thought, the proposed .volatile syntax possibilities are a little funny (even by the standards of the base strangeness of the proposed syntax): // one of these I guess? int[] foo = ...; int x = foo[0].volatile.compareAndSet(10, 15); // this one, I suppose? int x = foo.volatile[0].compareAndSet(10, 15); If .volatile is treated as a postfix unary operator which operates on the left-hand term (what precedence I wonder? probably pretty high), then the first option makes the most sense and aligns well with [objectexpression.]foo.volatile... where foo is a field. -- - DML
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote: nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? I would agree given you are now calling Character.toLowerCase (2) StringBuilder can be used to replace the StringBuffer in toString(). Agree, but I think this could be considered optional for this putback... -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: JDK 9 RFR to remove a raw lint warning from java/lang/invoke/MethodHandleImpl.java
Joe,
Looks good (not a Reviewer)!
Thanks for the fix.
Best regards,
Vladimir Ivanov
On 3/4/14 6:03 AM, Joe Darcy wrote:
Hello,
The recent changeset for
8027827: Improve performance of catchException combinator
8034120: MethodHandles.catchException doesn't handle
VarargsCollector right
introduced a raw lint warning to the core libraries. Please review the
patch below which removes the warning by adding ? extends Throwable
to the type the exType parameter of the method. A JDK build succeeds
with the new signature in place.
Thanks,
-Joe
diff -r 6cfedc362f48
src/share/classes/java/lang/invoke/MethodHandleImpl.java
--- a/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon Mar
03 18:17:00 2014 +0400
+++ b/src/share/classes/java/lang/invoke/MethodHandleImpl.java Mon Mar
03 17:49:52 2014 -0800
@@ -734,7 +734,7 @@
* (see {@link InvokerBytecodeGenerator#emitGuardWithCatch
emitGuardWithCatch}).
*/
@LambdaForm.Hidden
-static Object guardWithCatch(MethodHandle target, Class exType,
MethodHandle catcher,
+static Object guardWithCatch(MethodHandle target, Class? extends
Throwable exType, MethodHandle catcher,
Object... av) throws Throwable {
try {
return target.invokeWithArguments(av);
Re: RFR: JDK-8035340 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
On 05/03/2014 19:53, Mark Sheppard wrote: Hi Please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035340/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035340 This effort adds NULL checks and pending exception check after JNI calls and adds NULL check to malloc calls in WindowsPreferences.c This is a Windows change set. This mostly looks okay to me. One thing is the return path at L124 where I assume it should release valueNameStr. Could you also look at the indentation of return at L121 as it initially looks like later code is dead code. Otherwise looks okay. -Alan.
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On 05/03/2014 20:18, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Do you know if these tests exercise this code? I realize the Xerces didn't come with tests but I do have a concern about changing regex code without knowing that it is at least exercised by some tests. -Alan.
Re: RFR: JDK-8035340 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
Looks ok to me Mark. -Chris. On 05/03/2014 19:53, Mark Sheppard wrote: Hi Please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035340/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035340 This effort adds NULL checks and pending exception check after JNI calls and adds NULL check to malloc calls in WindowsPreferences.c This is a Windows change set. regards Mark
Re: JEP 193: Enhanced Volatiles
On 03/04/2014 03:16 PM, David M. Lloyd wrote:
On 03/03/2014 04:53 PM, David M. Lloyd wrote:
On 03/03/2014 04:25 PM, Brian Goetz wrote:
Posted: http://openjdk.java.net/jeps/193
Some follow-up thoughts on teasing apart the issues covered by this JEP.
There are three main layers of questions to answer:
1. How do we surface the various pieces of this into the programming
model? This includes language syntax (e.g.,
x.volatile.compareAndSet()), library surface (e.g., the fictitious and
not-terribly-satisfying VolatileInt interface), and relevant
restrictions (e.g., can we do volatile operations on non-volatile fields
or array elements?)
[...and then...]
2. Translation to bytecodes. What bytecode should javac emit when it
encounters an volatile accessor or atomic update? We've identified a
handful of candidates:
2a: new bytecodes. This is the most direct and pure, but most intrusive
(and least extensible), means of delivering on the promise of it's time
to move this stuff into the programming model proper. The wide
bytecode offers a means to express fenced variants of
{get,put}{field,static} with only a single new bytecode, but this
doesn't scale well to CAS (explosion of data types and data locations
(static field, instance field, array element)), which is really the
important case.
Somewhere between new bytecodes and field handles, perhaps a single
bytecode could suffice - one which is similar to getfield except instead
of reading the field, it pushes a virtual object on to the stack which
can then only be operated upon by invokeinterface on the pertinent
VolatileXxx interface and otherwise doesn't have a real type per se.
I suppose you'd also need a second bytecode for doing the same thing
with array elements like *aload, if that branch of this idea is pursued.
It could even be a *real* object if it were possible to rely on some
form of escape analysis to optimize it in the majority of cases, and if
the puzzle of allocation/caching/storing the object could be solved,
though the memory usage story in the non-escape case would have to be
pretty buttoned down to make this acceptable IMO.
--
- DML
RFR: (s) 8036095: RMI tests using testlibrary.RMID and testlibrary.JavaVM do not pass through vmoptions
Hi all, Please review this fix to change RMI's test library to pass through vmoptions and javaoptions to rmid and other JVM subprocesses. I've marked this as a small review because the changes of substance are confined to the first four files in the webrev, and they're pretty small. The remaining changes are to the individual security.policy files of each test, which requires different permissions granted in order to allow reading of the test.vm.opts and test.java.opts properties. Webrev: http://cr.openjdk.java.net/~smarks/reviews/8036095/webrev.0/ Bug report: https://bugs.openjdk.java.net/browse/JDK-8036095 Thanks, s'marks
Re: RFR: JDK-8035340 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
thanks Alan ... excellent spot (L124) regards Mark On 05/03/2014 21:33, Alan Bateman wrote: On 05/03/2014 19:53, Mark Sheppard wrote: Hi Please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035340/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035340 This effort adds NULL checks and pending exception check after JNI calls and adds NULL check to malloc calls in WindowsPreferences.c This is a Windows change set. This mostly looks okay to me. One thing is the return path at L124 where I assume it should release valueNameStr. Could you also look at the indentation of return at L121 as it initially looks like later code is dead code. Otherwise looks okay. -Alan.
Re: RFR: JDK-8035340 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
thanks Chris On 05/03/2014 21:50, Chris Hegarty wrote: Looks ok to me Mark. -Chris. On 05/03/2014 19:53, Mark Sheppard wrote: Hi Please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035340/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035340 This effort adds NULL checks and pending exception check after JNI calls and adds NULL check to malloc calls in WindowsPreferences.c This is a Windows change set. regards Mark
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On 3/5/2014 12:46 PM, Lance Andersen - Oracle wrote: On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote: nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? I would agree given you are now calling Character.toLowerCase Also, at line 463 and 464, it looks like 'uppers' meant to be 'lowers' defined at 462. (2) StringBuilder can be used to replace the StringBuffer in toString(). Agree, but I think this could be considered optional for this putback... Yes, feel free to replace StringBuffer with StringBuilder, Vector to ArrayList and etc. -Joe -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On 3/5/2014 1:38 PM, Alan Bateman wrote: On 05/03/2014 20:18, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Do you know if these tests exercise this code? I realize the Xerces didn't come with tests but I do have a concern about changing regex code without knowing that it is at least exercised by some tests. There are over 100 tests in SQE suite in this area, we'll take a look. -Joe -Alan.
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On Mar 5, 2014, at 5:10 PM, huizhe wang wrote: On 3/5/2014 12:46 PM, Lance Andersen - Oracle wrote: On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote: nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? I would agree given you are now calling Character.toLowerCase Also, at line 463 and 464, it looks like 'uppers' meant to be 'lowers' defined at 462. I think we should add tests with this change it looks like we really need to test this if your suggestion above is correct. (2) StringBuilder can be used to replace the StringBuffer in toString(). Agree, but I think this could be considered optional for this putback... Yes, feel free to replace StringBuffer with StringBuilder, Vector to ArrayList and etc. -Joe -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: JEP 193: Enhanced Volatiles
On 03/05/2014 02:37 PM, David M. Lloyd wrote: On 03/03/2014 04:25 PM, Brian Goetz wrote: Posted: http://openjdk.java.net/jeps/193 Some follow-up thoughts on teasing apart the issues covered by this JEP. There are three main layers of questions to answer: 1. How do we surface the various pieces of this into the programming model? This includes language syntax (e.g., x.volatile.compareAndSet()), library surface (e.g., the fictitious and not-terribly-satisfying VolatileInt interface), and relevant restrictions (e.g., can we do volatile operations on non-volatile fields or array elements?) Also, relatedly, how far is this taken? It is easy to imagine operations of this sort on reference fields and fields of all 8 primitive types, as well as arrays of all 8 primitive types, and object arrays (which would of course cover all arrays-of-arrays as well). Following the interface idea, you would need (at a maximum) one interface type for each primitive type and one interface type for Objects. Arrays should be able to reuse the same interface type since array element types are constrained to the same set of possible types as fields (though without an actual volatile qualifier of course). So 9 interface types at maximum. At minimum, just having interfaces for ints, longs, and Objects would match what is presently defined in j.u.c.atomic, so 3 seems to be the low end. Personally I think it would be nicest to support all 8, but I expect there were probably good reasons (I'm guessing relating to false sharing) that we don't have AtomicBooleanArrays and so forth. This should be all 8 primitive types. Other random thought, the proposed .volatile syntax possibilities are a little funny (even by the standards of the base strangeness of the proposed syntax): This should have been possibilities *for arrays* are a little funny... Guess I shouldn't bother sending emails with kids about. // one of these I guess? int[] foo = ...; int x = foo[0].volatile.compareAndSet(10, 15); // this one, I suppose? int x = foo.volatile[0].compareAndSet(10, 15); If .volatile is treated as a postfix unary operator which operates on the left-hand term (what precedence I wonder? probably pretty high), then the first option makes the most sense and aligns well with [objectexpression.]foo.volatile... where foo is a field. -- - DML
Re: JEP 193: Enhanced Volatiles
On 6/03/2014 3:22 AM, Remi Forax wrote: On 03/05/2014 06:07 PM, Peter Levart wrote: On 03/05/2014 05:55 PM, Jeroen Frijters wrote: Brian Goetz wrote: I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. And the JVM is the one implementing the language semantics (together with javac which feeds the JVM with bytecodes). Language semantcis are implemented by the combination of javac and JVM. If you say that this feature does not require any change to javac, you're just saying that you put all the burden on the JVM, but you *are* implementing the language semantics using annotations nevertheless... no, it's the other way around. javac maps Java the language to the bytecode semantics. The JVM only execute bytecodes. But if the JVM acts upon annotations to change how it executes those bytecodes then, as Peter said, the JVM is implementing the language semantics. It is also a bit misleading to say that the JVM only executes bytecodes, as the semantics of those bytecodes were driven by the requirements of the language. javac + jvm do form a team here. David Regards, Peter cheers, Rémi I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen
JDK 9 RFR of JDK-8036744: Fix raw lint warnings in java.lang.reflect.WeakCache
Hello,
Please review the simple patch below which resolves two unchecked cast
lint warnings in java.lang.reflect.WeakCache.
Thanks,
-Joe
diff -r 9099a251d211 src/share/classes/java/lang/reflect/WeakCache.java
--- a/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
11:53:35 2014 -0800
+++ b/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
16:51:20 2014 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -184,6 +184,7 @@
return reverseMap.size();
}
+@SuppressWarnings(unchecked) // refQueue.poll actually returns
CacheKeyK
private void expungeStaleEntries() {
CacheKeyK cacheKey;
while ((cacheKey = (CacheKeyK)refQueue.poll()) != null) {
@@ -351,6 +352,7 @@
}
@Override
+@SuppressWarnings(unchecked)
public boolean equals(Object obj) {
K key;
return obj == this ||
@@ -359,7 +361,7 @@
// cleared CacheKey is only equal to itself
(key = this.get()) != null
// compare key by identity
- key == ((CacheKeyK) obj).get();
+ key == ((CacheKeyK) obj).get(); // Cast is safe
from getClass check
}
void expungeFrom(ConcurrentMap?, ? extends ConcurrentMap?,
? map,
Re: JDK 9 RFR of JDK-8036744: Fix raw lint warnings in java.lang.reflect.WeakCache
+1
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPad
On Mar 5, 2014, at 7:52 PM, Joe Darcy joe.da...@oracle.com wrote:
Hello,
Please review the simple patch below which resolves two unchecked cast lint
warnings in java.lang.reflect.WeakCache.
Thanks,
-Joe
diff -r 9099a251d211 src/share/classes/java/lang/reflect/WeakCache.java
--- a/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
11:53:35 2014 -0800
+++ b/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
16:51:20 2014 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -184,6 +184,7 @@
return reverseMap.size();
}
+@SuppressWarnings(unchecked) // refQueue.poll actually returns
CacheKeyK
private void expungeStaleEntries() {
CacheKeyK cacheKey;
while ((cacheKey = (CacheKeyK)refQueue.poll()) != null) {
@@ -351,6 +352,7 @@
}
@Override
+@SuppressWarnings(unchecked)
public boolean equals(Object obj) {
K key;
return obj == this ||
@@ -359,7 +361,7 @@
// cleared CacheKey is only equal to itself
(key = this.get()) != null
// compare key by identity
- key == ((CacheKeyK) obj).get();
+ key == ((CacheKeyK) obj).get(); // Cast is safe from
getClass check
}
void expungeFrom(ConcurrentMap?, ? extends ConcurrentMap?, ? map,
Re: RFR: (s) 8036095: RMI tests using testlibrary.RMID and testlibrary.JavaVM do not pass through vmoptions
On 6/03/2014 8:05 AM, Stuart Marks wrote: Hi all, Please review this fix to change RMI's test library to pass through vmoptions and javaoptions to rmid and other JVM subprocesses. Are you sure you want to do that? Run everything with -Xcomp for example? David I've marked this as a small review because the changes of substance are confined to the first four files in the webrev, and they're pretty small. The remaining changes are to the individual security.policy files of each test, which requires different permissions granted in order to allow reading of the test.vm.opts and test.java.opts properties. Webrev: http://cr.openjdk.java.net/~smarks/reviews/8036095/webrev.0/ Bug report: https://bugs.openjdk.java.net/browse/JDK-8036095 Thanks, s'marks
Re: JDK 9 RFR of JDK-8036744: Fix raw lint warnings in java.lang.reflect.WeakCache
Thumbs up.
Mandy
On 3/5/2014 4:52 PM, Joe Darcy wrote:
Hello,
Please review the simple patch below which resolves two unchecked cast
lint warnings in java.lang.reflect.WeakCache.
Thanks,
-Joe
diff -r 9099a251d211 src/share/classes/java/lang/reflect/WeakCache.java
--- a/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
11:53:35 2014 -0800
+++ b/src/share/classes/java/lang/reflect/WeakCache.javaWed Mar 05
16:51:20 2014 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -184,6 +184,7 @@
return reverseMap.size();
}
+@SuppressWarnings(unchecked) // refQueue.poll actually returns
CacheKeyK
private void expungeStaleEntries() {
CacheKeyK cacheKey;
while ((cacheKey = (CacheKeyK)refQueue.poll()) != null) {
@@ -351,6 +352,7 @@
}
@Override
+@SuppressWarnings(unchecked)
public boolean equals(Object obj) {
K key;
return obj == this ||
@@ -359,7 +361,7 @@
// cleared CacheKey is only equal to itself
(key = this.get()) != null
// compare key by identity
- key == ((CacheKeyK) obj).get();
+ key == ((CacheKeyK) obj).get(); // Cast is safe
from getClass check
}
void expungeFrom(ConcurrentMap?, ? extends ConcurrentMap?,
? map,
Re: RFR: (s) 8036095: RMI tests using testlibrary.RMID and testlibrary.JavaVM do not pass through vmoptions
On 3/5/14 4:57 PM, David Holmes wrote: On 6/03/2014 8:05 AM, Stuart Marks wrote: Please review this fix to change RMI's test library to pass through vmoptions and javaoptions to rmid and other JVM subprocesses. Are you sure you want to do that? Run everything with -Xcomp for example? The choice is between not passing the options and passing them through. I think it's more surprising not to have the options passed through. For example, the test makefiles pass -ea and -esa to jtreg, which adds them to test.vm.opts. Rmid and its activation group JVMs are just as much part of the system under test as the JDK running the test, so not having these options passed through means that we've been testing without assertions enabled in the subprocesses all these years. The -D option to set a system property should probably be passed through as well. I don't know if this makes sense for -Xcomp though. s'marks
Re: RFR: (s) 8036095: RMI tests using testlibrary.RMID and testlibrary.JavaVM do not pass through vmoptions
On 6/03/2014 3:40 PM, Stuart Marks wrote: On 3/5/14 4:57 PM, David Holmes wrote: On 6/03/2014 8:05 AM, Stuart Marks wrote: Please review this fix to change RMI's test library to pass through vmoptions and javaoptions to rmid and other JVM subprocesses. Are you sure you want to do that? Run everything with -Xcomp for example? The choice is between not passing the options and passing them through. I think it's more surprising not to have the options passed through. For example, the test makefiles pass -ea and -esa to jtreg, which adds them to test.vm.opts. Rmid and its activation group JVMs are just as much part of the system under test as the JDK running the test, so not having these options passed through means that we've been testing without assertions enabled in the subprocesses all these years. The -D option to set a system property should probably be passed through as well. I don't know if this makes sense for -Xcomp though. Indeed that is the problem with all of this - some settings you will want to pass through and some you won't. Though perhaps -Xcomp and other stress options are not something you have to be concerned about with normal core-libs testing. And conversely hotspot testing is unlikely to be running the RMI tests. So I guess it should be reasonably safe. BTW not a review (sorry) :) David s'marks
RE: JEP 193: Enhanced Volatiles
Sorry Peter, but that is a nonsensical definition of language semantics. By using your definition, if Microsoft adds a feature to the CLR using custom attributes, the Java language semantics change, because on IKVM you can use these custom attributes in your Java code. Regards, Jeroen -Original Message- From: Peter Levart [mailto:peter.lev...@gmail.com] Sent: Wednesday, March 5, 2014 18:07 To: Jeroen Frijters; Brian Goetz; core-libs-dev@openjdk.java.net; Doug Lea Subject: Re: JEP 193: Enhanced Volatiles On 03/05/2014 05:55 PM, Jeroen Frijters wrote: Brian Goetz wrote: I suspect you were expecting this response: we don't add language semantics through annotations. Technically, we're not adding language semantics. The JVM is the one interpreting the annotations. And the JVM is the one implementing the language semantics (together with javac which feeds the JVM with bytecodes). Language semantcis are implemented by the combination of javac and JVM. If you say that this feature does not require any change to javac, you're just saying that you put all the burden on the JVM, but you *are* implementing the language semantics using annotations nevertheless... Regards, Peter I'm not trying to frustrate you; evolving a language with millions of users is really, really hard. And one of the things that makes it hard is recognizing our intrinsic conflicts of interest between what good will this do me and what harm will it do others. I understand, that's why I want to avoid adding language support for this niche/specialist feature. Regards, Jeroen
