Re: CFR - updated 8001667: Comparator combinators and extension methods
Hi Henry, Minor thing. In Comparator: 194 * @param other the other comparator used when equals on this. 195 * @throws NullPointerException if the argument is null. 196 * @since 1.8 197 */ 198 default ComparatorT thenComparing(Comparator? super T other) { 199 return Comparators.compose(this, other); 200 } Perhaps: @param other the other comparator to be used when this comparator compares two objects that are equal @throws NullPointerException if the argument is null. @since 1.8 @return A lexicographic order comparator composed of this and then the other comparator In Comparators: 241 * @param T the element type to be compared 242 * @param first the first comparator 243 * @param second the secondary comparator used when equals on the first 244 */ 245 public staticT ComparatorT compose(Comparator? super T first, Comparator? super T second) { 246 Objects.requireNonNull(first); 247 Objects.requireNonNull(second); 248 return (ComparatorT Serializable) (c1, c2) - { 249 int res = first.compare(c1, c2); 250 return (res != 0) ? res : second.compare(c1, c2); 251 }; 252 } @param second the second comparator to be used when the first comparator compares two objects that are equal @return A lexicographic order comparator composed of the first and then the second comparator Paul. On Mar 5, 2013, at 8:46 PM, Henry Jen henry@oracle.com wrote: Hi, Another update to reflect functional interface renames involved in the API, and a bug fix for a regression found earlier. CCC had been approved. Can we get it reviewed and pushed? [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev Cheers, Henry
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types in collections should be generally discouraged but having useful equals() and hashCode() is ever so convenient. http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/ Mike
hg: jdk8/tl/jdk: 8009397: test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Changeset: 34372bb9115d Author:sla Date: 2013-03-05 19:25 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/34372bb9115d 8009397: test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket Reviewed-by: alanb ! src/share/back/transport.c ! src/share/demo/jvmti/hprof/hprof_init.c ! src/solaris/back/linker_md.c ! src/solaris/demo/jvmti/hprof/hprof_md.c ! src/windows/back/linker_md.c ! src/windows/demo/jvmti/hprof/hprof_md.c
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote: Really, this is a lot of fuss over nothing. There is actually no fundamental difference between Scala's Option, Guava's Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are modelling the same thing, the possibility of a value not being present. The fact that there may be minor differences in api or semantics around whether null is a legal value are minor in the scheme of things (and yes, null is a pretty stupid legal value of a Some IMHO). Stephen's example is ludicrous, why have a list of optional values? You'd flatten down into just a list – and an optional list only makes sense if the enclosed list is guaranteed to be non-empty, otherwise you just return an empty list! People like shooting their own feet. http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis If we are going to use potential straw-men as arguments we can stall all progress. Please concentrate on the important matters, let's disavow null as a valid value and save us all a billion dollars Also Scala Option is not the only way to solve the null problem. The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and IntelliJ at least. . cheers, jed. cheers, Rémi On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote: Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types in collections should be generally discouraged but having useful equals() and hashCode() is ever so convenient. http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/ Mike
Re: Request for review- RFE 8005716
On 06/03/2013 04:13, 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_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Dean makes a good point, the @8 need be at the end to match the decoration scheme. Also I think it's 32-bit only although it should just not be found on 64-bit so it will skip on JNI_OnLoad_L. -Alan.
Re: Request for review- RFE 8005716
On 05/03/2013 23:05, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. Overall I think this is quite good and follows the proposal in the JEP. I don't see anything obvious wrong with the logic and everything should just work for shared libraries as it does today. I assume you'll run all the appropriate tests. I see Dean's suggestion to add a JVM function to allow for a lookup table when the symbol information is available, If you do that then you'll need to get the hotspot changes in first. Alternatively, change what you have later once the hotspot changes are in.Just on the As findBuiltJniFunction can locate a built-in or a JNI_OnLoad/OnUnload in a library then the function name is probably not quite right (shouldn't have Builtin in the name). A minor nit in _findBuiltinLib is that if the OOME path should call JNU_ReleaseStringPlatformChars before returning. There are a few commented out statements in jni_util_md.c that I assume will be removed. Also you might want to check the indentation in both getProcessHandle implementations as they look like 2-spaces whereas the libs uses 4 (although this may be mute if you use a JVM function). Otherwise I think this is good and I can sponsor the jdk part to this and help get it into jdk8/tl. -Alan
RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()
Hi, Can I get a review of this small fix for issue 8007808: Missing method: Executable.getAnnotatedReturnType() http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/ When we added Core Reflection support for type annotations this method got left out on Executable, but were included on Method and Constructor. For Oracle reviewers, CCC is approved. cheers /Joel
Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()
Hi Joel, Look good; approved. -Joe On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote: Hi, Can I get a review of this small fix for issue 8007808: Missing method: Executable.getAnnotatedReturnType() http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/ When we added Core Reflection support for type annotations this method got left out on Executable, but were included on Method and Constructor. For Oracle reviewers, CCC is approved. cheers /Joel
hg: jdk8/tl/langtools: 3 new changesets
Changeset: d0178bd8125c Author:mcimadamore Date: 2013-03-06 15:29 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d0178bd8125c 8009299: Javac crashes when compiling method reference to static interface method Summary: Assertion in Check.checMethod is too strict Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/jvm/Pool.java + test/tools/javac/lambda/MethodReference66.java Changeset: 8a78243291ef Author:mcimadamore Date: 2013-03-06 15:33 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/8a78243291ef 8009459: Wrong behavior of diamond finder with source level 7 Summary: Diamond finder doesn't take into account different inference behaviors Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/Attr.java + test/tools/javac/generics/diamond/6939780/T6939780.java + test/tools/javac/generics/diamond/6939780/T6939780_7.out + test/tools/javac/generics/diamond/6939780/T6939780_8.out - test/tools/javac/generics/diamond/T6939780.java - test/tools/javac/generics/diamond/T6939780.out Changeset: c98b3e96c726 Author:mcimadamore Date: 2013-03-06 15:33 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/c98b3e96c726 8009391: Synthetic name of serializable lambda methods should not contain negative numbers Summary: Use hex representation of method signature hashcode to avoid negative numbers Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
Just another idea: findFirst() throws NoSuchElementException if the stream is empty. Add an intermediary operation that maps an empty stream to a non-empty one StreamT ifEmpty(T item); so we can say stream.ifEmpty(null).findFirst(); stream.ifEmpty(0).reduce(sum); stream.ifEmpty(-1).min(); Zhong Yu On Wed, Mar 6, 2013 at 3:47 AM, Remi Forax fo...@univ-mlv.fr wrote: Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types in collections should be generally discouraged but having useful equals() and hashCode() is ever so convenient. http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/ Mike
Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()
Hi Joel, looks good for me too :) Rémi On 03/06/2013 04:35 PM, Joe Darcy wrote: Hi Joel, Look good; approved. -Joe On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote: Hi, Can I get a review of this small fix for issue 8007808: Missing method: Executable.getAnnotatedReturnType() http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/ When we added Core Reflection support for type annotations this method got left out on Executable, but were included on Method and Constructor. For Oracle reviewers, CCC is approved. cheers /Joel
Re: Request for review- RFE 8005716
On Mar 5, 2013, at 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_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Good catch Dean. Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Bill has already factored out the implementation of getProcessHandle. Although your approach is cleaner, I'm concerned about creating a VM version dependency. For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. If they didn't do that, we would have already gone down that path. The jdk sources already contain several instances of dlopen, dlysym and the Windows equivalents so I don't see a need to add a new VM function. Bob. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
hg: jdk8/tl: 8009162: root repo make test target should run against image
Changeset: b35d986ff276 Author:mduigou Date: 2013-03-06 08:37 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/b35d986ff276 8009162: root repo make test target should run against image Reviewed-by: alanb, martin, erikj ! common/makefiles/Main.gmk
Re: RFR: 8006593: Performance and compatibility improvements to hash based Map implementations
On Mar 6 2013, at 09:19 , Alan Bateman wrote: On 05/03/2013 22:46, Mike Duigou wrote: I have updated the webrev to remove the useAltHashing boolean. http://cr.openjdk.java.net/~mduigou/JDK-8006593/5/webrev/ Mike Peter's suggestion to remove the useAltHashing field is a great idea. I've looked at the webrev and it looks good to me. The logical xor in initHashSeedAsNeeded is subtle. As this is for jdk7u only then I assume you'll get approval on jdk7u-dev once you are done here. Yes. Per the original review request: Once review is completed here this patch will be proposed to JDK7u-dev for integration into the next 7u performance/feature release. I am going to leave this review open for another 24 hours before requesting approval from JDK7u-dev. Mike
Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use
On 3/5/2013 9:16 AM, Jim Gish wrote: On 03/01/2013 05:48 PM, Mandy Chung wrote: On 3/1/2013 1:25 PM, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ This removes the stack search from Logger.findResourceBundle() It's good to see this stack walk search of resource bundle going away. In Logger.java, the existing implementation returns the previous cached resource bundle if it fails to find one matching the current locale but the name matches: 1608 if (name.equals(catalogName)) { 1609 // Return the previous cached value for that name. 1610 // This may be null. 1611 return catalog; 1612 } Your fix removes these lines which I think is fine. The Logger.getResourceBundle method specifies to return the resource bundle for this logger for the current default locale. I think it'd be rare to change the default locale during the execution of an application. The old behavior upon reaching this point in the code was as follows: To get here, the sought after bundle was not found and (from the checks on line 1559,1560), either (1) the previously cached catalog was null, meaning nothing was yet cached and so null was returned, or correct. (2) the current locale had changed and no bundle for that locale was found, in which case the cached bundle (for the old/wrong locale) was returned, or do you mean it returns the cached bundle only if the name matches? or am I missing the code that you read? (3) the name was the same but the location of the objects used to compare the cached name with that passed in was different, Can you elaborate? Are you referring the if (name.equals(catalogName)) statement? so the previously cached bundle was returned (this is frankly an odd case, because it also means that re-searching for a previously found bundle is now failing, which only seems possible if the bundle is/was (a) not available via the system classloader, (b) the context classloader, or (c), the classloaders up the call chain. The new behavior /does /still allow for a change in the default locale. For example, if you first call findResourceBundleName(foo) and the default locale is US, it will search for the foo_US bundle and set catalogLocale=US, and catalogName=foo and return the foo_US bundle. If you then search for foo and have changed the default locale to DE (if that is indeed a valid locale), it will then search for foo_DE and if found set catalogLocale=DE and cache and return the foo_DE bundle. The /only /change in behavior that I see here, is that if you change the locale and the bundle is not found, null will be returned instead of the previously cached bundle (if one exists), for a /different/ locale. Right. The behavioral difference that I point out was when foo_DE is cached, later the current locale is set to JP where foo_JP and foo don't exist, the old behavior returns foo_DE which was a bug too me whereas the new behavior returns null which matches the spec of Logger.getResourceBundle(). So, the old behavior in effect had a notion of a default catalog, but only if you happened to find a bundle on a previous lookup and cache it. However, you wouldn't be guaranteed of acquiring this same bundle on a subsequent search if a search for a different name had intervened. Thus, the new behavior is that if you search for a name/locale and it's not found, you will get null (every time). This seems better to me because it's consistent, explainable and simple. This matches what Logger.getResourceBundle() is specified and the behavior is correct. The difference in behavior is a minimal compatibility risk. Comments on the updated webrev: http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ L122-125: you may want to replace code/code with {@code } L129-130: you can use @linkplain instead: {@linkplain ClassLoader#getSystemClassLoader()system ClassLoader} L141-142: the bundle associated with the Logger object itself cannot be changed - is this statement correct? The cached catalog object is not accessed directly; instead when it finds the resource bundle every time it logs a message. In addition the new paragraph L132-142 doesn't seem to be necessary since the spec is pretty clear on using the resource bundle of the current locale. L1575: the @link here is not necessary in a comment LoggerResourceBundleRace.java: I think what you really want is to add a new test that sets a context class loader to a class loader that finds the resource bundle for a logger that a system class loader can't find. I suggest to leave this test as it is and then add a new test to exercise the context class loader search of a resource bundle as a separate RFE that will improve the test coverage. Mandy I'd appreciate you verifying this. Thanks, Jim I suggest to document this behavioral
Re: Request for review- RFE 8005716
On Mar 6 2013, at 08:21 , Bob Vandette wrote: For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. In earlier days the jni.h file was copied from hotspot into jdk during the compilation process. The current build process doesn't validate that the two files match which opens up opportunities for inconsistencies and problems. If they didn't do that, we would have already gone down that path. If it's possible to improve upon having two jni.h instances then we should consider doing so. Mike
hg: jdk8/tl/jdk: 8006853: OCSP timeout set to wrong value if com.sun.security.ocsp.timeout 0
Changeset: 7246a6e4dd5a Author:juh Date: 2013-02-28 16:36 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7246a6e4dd5a 8006853: OCSP timeout set to wrong value if com.sun.security.ocsp.timeout 0 Reviewed-by: mullan ! src/share/classes/sun/security/provider/certpath/OCSP.java
Re: Request for review- RFE 8005716
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_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). I'll remove that. I redid the call to findBuiltinJniFunction but forgot to remove that. Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() an exported function in the VM? How does it find JNI_OnLoad_L? For Windows and Linux, it would use basically your same code, just refactored a little. By the way, did you consider using the special RTLD_DEFAULT handle instead of dlopen(NULL)? Via a table setup by the developer/build system when the library is linked in? Yes, in previous projects we did exactly that for more exotic OSes. dl bill dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
Whatever happened to the elvis operator solution from project coin, surely that solves the majority of issues Optional was meant to solve anyway, without the headache of type-mix mud clutering up the code. http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/47.html _ From: Remi Forax [mailto:fo...@univ-mlv.fr] To: Jed Wesley-Smith [mailto:jed.wesleysm...@gmail.com] Cc: lambda-libs-spec-expe...@openjdk.java.net [mailto:lambda-libs-spec-expe...@openjdk.java.net], lambda-...@openjdk.java.net [mailto:lambda-...@openjdk.java.net], core-libs-dev@openjdk.java.net [mailto:core-libs-dev@openjdk.java.net] Sent: Wed, 06 Mar 2013 11:58:43 + Subject: Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote: Really, this is a lot of fuss over nothing. There is actually no fundamental difference between Scala's Option, Guava's Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are modelling the same thing, the possibility of a value not being present. The fact that there may be minor differences in api or semantics around whether null is a legal value are minor in the scheme of things (and yes, null is a pretty stupid legal value of a Some IMHO). Stephen's example is ludicrous, why have a list of optional values? You'd flatten down into just a list – and an optional list only makes sense if the enclosed list is guaranteed to be non-empty, otherwise you just return an empty list! People like shooting their own feet. http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis If we are going to use potential straw-men as arguments we can stall all progress. Please concentrate on the important matters, let's disavow null as a valid value and save us all a billion dollars Also Scala Option is not the only way to solve the null problem. The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and IntelliJ at least. . cheers, jed. cheers, Rémi On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote: Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not
RE: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations
My intention was to say implementation removed. Uwe - Uwe Schindler uschind...@apache.org Apache Lucene PMC Member / Committer Bremen, Germany http://lucene.apache.org/ -Original Message- From: Joe Darcy [mailto:joe.da...@oracle.com] Sent: Monday, March 04, 2013 5:16 PM To: Uwe Schindler Cc: 'Core-Libs-Dev'; 'Jonathan Gibbons'; 'Alexander Buckley'; 'Maurizio Cimadamore' Subject: Re: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations On 03/02/2013 02:34 PM, Uwe Schindler wrote: Looks good to me! Are this the only methods in the corelib that were removed in favour of a default implementation in the interface? I would not described the method as removed, but to my knowledge AnnotatedElement.isAnnotationPresent was the only existing interface method in the core libraries replaced by a default method. The intended use case for default methods is adding new methods to interface and that change has certainly been done in other parts of the API. Cheers, -Joe Uwe - Uwe Schindler uschind...@apache.org Apache Lucene PMC Member / Committer Bremen, Germany http://lucene.apache.org/ -Original Message- From: Joe Darcy [mailto:joe.da...@oracle.com] Sent: Saturday, March 02, 2013 3:06 AM To: Core-Libs-Dev Cc: Jonathan Gibbons; Alexander Buckley; Maurizio Cimadamore; uschind...@apache.org Subject: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations Hello, The changes pushed under 8007113: Upgrade AnnotatedElement.isAnnotionPresent to be a default method http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e04467fa13af combined with how javac currently models default methods under pre-JDK-8 source versions can result in some source compatibility impacts for those using JDK 8 to compile under older source versions if the recommended practice of setting the bootclasspath is not followed. [1] To mitigate these impacts, I'm proposing the changes in 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations http://cr.openjdk.java.net/~darcy/8009267.1/ which have the effect of restoring a concrete isAnnotationPresent method to Class, Package, Field, Method, and Constructor. The implementation delegates to the default method on the AnnotatedElement interface. Thanks, -Joe [1] http://mail.openjdk.java.net/pipermail/compiler-dev/2013- February/005738.html
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
There is no need for an Option container to show how nested collections may be misused, you could just as easily show an example of a ListListListT that is isomorphic to a flattened IterableT . I'd simply point to the utility of the monadic bind or flatMap function! There are several reasons why Option types provide serious usability improvements over null, notwithstanding annotation support. The first is that the type-system prevents misuse (modulo unsafe API), and the second is that it allows operations that produce optional results to be sequenced easily without if checks. For mor information see previous posts on Optional forming a Monad. cheers, jed. On 06/03/2013, at 10:58 PM, Remi Forax fo...@univ-mlv.fr wrote: On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote: Really, this is a lot of fuss over nothing. There is actually no fundamental difference between Scala's Option, Guava's Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are modelling the same thing, the possibility of a value not being present. The fact that there may be minor differences in api or semantics around whether null is a legal value are minor in the scheme of things (and yes, null is a pretty stupid legal value of a Some IMHO). Stephen's example is ludicrous, why have a list of optional values? You'd flatten down into just a list – and an optional list only makes sense if the enclosed list is guaranteed to be non-empty, otherwise you just return an empty list! People like shooting their own feet. http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis If we are going to use potential straw-men as arguments we can stall all progress. Please concentrate on the important matters, let's disavow null as a valid value and save us all a billion dollars Also Scala Option is not the only way to solve the null problem. The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and IntelliJ at least. . cheers, jed. cheers, Rémi On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote: Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types
Re: Request for review- RFE 8005716
Hi Bill; Some notes from reviewing the JDK side changes. System.java/Runtime.java: The example which begins with the name If the filename argument, needs to better identify that L is an example. (Italics?) java/lang/ClassLoader.java: NativeLibrary::fromClass could be final. ClassLoader.c: In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib() These two values are known at compile time. int prefixLen = (int) strlen(JNI_LIB_PREFIX); int suffixLen = (int) strlen(JNI_LIB_SUFFIX); Some of the error conditions don't throw exceptions. Such as: if (cname == NULL) { return NULL; } The prefix and suffix are stripped from cname without checking that cname actually contains the prefix or suffix. if (len prefixLen) is invariant. src/solaris/native/common/jni_util_md.c: void* getProcessHandle() { static void* procHandle = NULL; if (procHandle == NULL) { procHandle = (void*)dlopen(NULL, RTLD_LAZY); } return procHandle; } Why is the error handling code commented out? Mike On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong
Really, this is a lot of fuss over nothing. There is actually no fundamental difference between Scala's Option, Guava's Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are modelling the same thing, the possibility of a value not being present. The fact that there may be minor differences in api or semantics around whether null is a legal value are minor in the scheme of things (and yes, null is a pretty stupid legal value of a Some IMHO). Stephen's example is ludicrous, why have a list of optional values? You'd flatten down into just a list – and an optional list only makes sense if the enclosed list is guaranteed to be non-empty, otherwise you just return an empty list! If we are going to use potential straw-men as arguments we can stall all progress. Please concentrate on the important matters, let's disavow null as a valid value and save us all a billion dollars. cheers, jed. On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote: Ok, let be nuclear on this, There is no good reason to introduce OptionalT in java.util. It doen't work like Google's Guava Optional despite having the same name, it doesn't work like Scala's Option despite having a similar name, moreover the lambda pipeline face a similar issue with the design of collectors (see stream.collect()) but solve that similar problem with a different design, so the design of Optional is not even consistent with the rest of the stream API. So why do we want something like Optional, we want it to be able to represent the fact that as Mike states a returning result can have no value by example Colections.emptyList().stream().findFirst() should 'return' no value. As Stephen Colebourne said, Optional is a bad name because Scala uses Option [1] which can used in the same context, as result of a filter/map etc. but Option in Scala is a way to mask null. Given the name proximity, people will start to use Optional like Option in Scala and we will see methods returning things like OptionalListOptionalString. Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed implementation, this will generate a lot of confusions and frustrations. In fact, we don't need Optional at all, because we don't need to return a value that can represent a value or no value, the idea is that methods like findFirst should take a lambda as parameter letting the user to decide what value should be returned by findFirst if there is a value and if there is no value. So instead of stream.findFirst().orElse(null) you will write stream.findFirst(orNull) with orNull() defined as like that public static T Optionalizer orNull() { return (isPresent, element) - isPresent? element: null; } The whole design is explained here [2] and is similar to the way Collectors are defined [3], it's basically the lambda way of thinking, instead of creating an object representing the different states resulting of a call to findFirst, findFirst takes a lambda as parameter which is fed with the states of a call. cheers, Rémi [1] http://www.scala-lang.org/api/current/index.html#scala.Option [2] http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html [3] http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java On 03/04/2013 09:29 PM, Mike Duigou wrote: Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types in collections should be generally discouraged but having useful equals() and hashCode() is ever so convenient. http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/ Mike
Re: CFR - updated 8001667: Comparator combinators and extension methods
Hello, I'm not one of the people that you're looking at to review this, but I have to give this feedback anyway. I tried to give similar feedback on another mailing list and got no response (maybe I chose the wrong list). These changes have been bothering me. :) 1. Why disable the following code even though it is (theoretically) safe? ComparatorCharSequence a = comparing(CharSequence::length); ComparatorString b = a.thenComparing(CASE_INSENSITIVE_ORDER); That code would compile if the signatures of the thenComparing methods had generics like S extends T instead of T. The example above is conjured up and ridiculous, but I think real code will have use for it. Is there any downside to narrowing the return type this way? 2. There's a thenComparing(Function), but no thenComparing(Function, Comparator). This seems like a big omission. Surely people will want secondary orderings on fields by something other than natural (null-unfriendly) ordering. Also, a Comparators.comparing(Function, Comparator) method would remove the need for all the Map-centric methods that are currently in the Comparators class. For instance: comparing(Map.Entry::getValue, naturalOrder()). 3. There is no support for sorting of nullable values or fields. Sorting of nullable values directly perhaps should not be encouraged, but sorting values by nullable fields within a chained sort is completely reasonable. Please add some support for this, either as chain methods on Comparator, or as factory methods on Comparators. 4. If Comparator had min(a,b) and max(a,b) methods, the Comparators.lesserOf(c) and greaterOf(c) methods would be unnecessary. The min/max methods would be generally more useful than the BinaryOperators returned from Comparators, and c::min reads better than Comparators.lesserOf(c). 5. Comparators.reverseOrder(c) is confusing in that it has different behavior than Collections.reverseOrder(c). It throws an NPE. Also, this new method seems to be useless because one could call c.reverseOrder() instead. I suggest removing the method. 6. I don't see why Comparators.compose(c1,c2) is useful given that users can call c1.thenComparing(c2). The latter reads better; the word compose does not naturally fit with comparators and has strange connotations for those with Math backgrounds. 7. Last I checked, even this simple example did not compile: ComparatorString c = comparing(String::length); It was confused about whether I was implying a ToDoubleFunction or a ToLongFunction, which were both wrong. I also ran into a lot of type inference loop problems when chaining. Is this simply a problem with lambda evaluation that's going to be fixed before Java 8 is officially released? Is there something more complex going on here that makes statements like this impossible? If the compilation problems aren't going to be fixed prior to the release, or if they cannot be fixed, then none of these Comparator/Comparators additions are useful. You would be better off removing them. -Michael On Tue, Mar 5, 2013 at 11:46 AM, Henry Jen henry@oracle.com wrote: Hi, Another update to reflect functional interface renames involved in the API, and a bug fix for a regression found earlier. CCC had been approved. Can we get it reviewed and pushed? [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev Cheers, Henry
Re: CFR - updated 8001667: Comparator combinators and extension methods
Hi, just one suggestion: rename comparing with compareWith 1) public static T, U extends Comparable? super U ComparatorT compareWith(Function? super T, ? extends U keyExtractor) { 2) default ComparatorT thenCompareWith(Comparator? super T other) Best Regards, Ali Ebrahimi On Wed, Mar 6, 2013 at 12:16 AM, Henry Jen henry@oracle.com wrote: Hi, Another update to reflect functional interface renames involved in the API, and a bug fix for a regression found earlier. CCC had been approved. Can we get it reviewed and pushed? [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev Cheers, Henry
Re: Request for review- RFE 8005716
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
RE: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations
Looks good to me! Are this the only methods in the corelib that were removed in favour of a default implementation in the interface? Uwe - Uwe Schindler uschind...@apache.org Apache Lucene PMC Member / Committer Bremen, Germany http://lucene.apache.org/ -Original Message- From: Joe Darcy [mailto:joe.da...@oracle.com] Sent: Saturday, March 02, 2013 3:06 AM To: Core-Libs-Dev Cc: Jonathan Gibbons; Alexander Buckley; Maurizio Cimadamore; uschind...@apache.org Subject: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations Hello, The changes pushed under 8007113: Upgrade AnnotatedElement.isAnnotionPresent to be a default method http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e04467fa13af combined with how javac currently models default methods under pre-JDK-8 source versions can result in some source compatibility impacts for those using JDK 8 to compile under older source versions if the recommended practice of setting the bootclasspath is not followed. [1] To mitigate these impacts, I'm proposing the changes in 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations http://cr.openjdk.java.net/~darcy/8009267.1/ which have the effect of restoring a concrete isAnnotationPresent method to Class, Package, Field, Method, and Constructor. The implementation delegates to the default method on the AnnotatedElement interface. Thanks, -Joe [1] http://mail.openjdk.java.net/pipermail/compiler-dev/2013- February/005738.html
hg: jdk8/tl: 8009019: Updates to generated-configure.sh required for 8008914
Changeset: cb0ac8979caa Author:tbell Date: 2013-02-26 09:25 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/cb0ac8979caa 8009019: Updates to generated-configure.sh required for 8008914 Reviewed-by: sundar, jlaskey, jjg ! common/autoconf/generated-configure.sh
Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()
Thanks for the reviews Joe and Remi cheers /Joel On 6 mar 2013, at 17:15, Remi Forax fo...@univ-mlv.fr wrote: Hi Joel, looks good for me too :) Rémi On 03/06/2013 04:35 PM, Joe Darcy wrote: Hi Joel, Look good; approved. -Joe On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote: Hi, Can I get a review of this small fix for issue 8007808: Missing method: Executable.getAnnotatedReturnType() http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/ When we added Core Reflection support for type annotations this method got left out on Executable, but were included on Method and Constructor. For Oracle reviewers, CCC is approved. cheers /Joel
Re: Review: 7032154 - Performance tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal
The link below has been updated with a few minor changes, notably to use constants from {Double,Float}Consts and to include the link to the OpenJDK issue report. A formatting issue resulting from an awk failure during webrev script execution was also fixed. B. On Feb 28, 2013, at 1:34 PM, Brian Burkhalter wrote: I forgot that the attachment is not redistributed and that I can now post on cr.openjdk.java.net so here's the webrev: http://cr.openjdk.java.net/~bpb/7032154/ Begin forwarded message: This concerns the issue http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7032154. The patch described in the issue has been updated to be with respect to the current JDK repository tip. The updated version is attached to this message.
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; == Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug:http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use
On 03/06/2013 12:58 PM, Mandy Chung wrote: On 3/5/2013 9:16 AM, Jim Gish wrote: On 03/01/2013 05:48 PM, Mandy Chung wrote: On 3/1/2013 1:25 PM, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ This removes the stack search from Logger.findResourceBundle() It's good to see this stack walk search of resource bundle going away. In Logger.java, the existing implementation returns the previous cached resource bundle if it fails to find one matching the current locale but the name matches: 1608 if (name.equals(catalogName)) { 1609 // Return the previous cached value for that name. 1610 // This may be null. 1611 return catalog; 1612 } Your fix removes these lines which I think is fine. The Logger.getResourceBundle method specifies to return the resource bundle for this logger for the current default locale. I think it'd be rare to change the default locale during the execution of an application. The old behavior upon reaching this point in the code was as follows: To get here, the sought after bundle was not found and (from the checks on line 1559,1560), either (1) the previously cached catalog was null, meaning nothing was yet cached and so null was returned, or correct. (2) the current locale had changed and no bundle for that locale was found, in which case the cached bundle (for the old/wrong locale) was returned, or do you mean it returns the cached bundle only if the name matches? or am I missing the code that you read? Yes. (3) the name was the same but the location of the objects used to compare the cached name with that passed in was different, Can you elaborate? Are you referring the if (name.equals(catalogName)) statement? It used to be: if (name == catalogName) and now is if (name.equals(catalogName)) Therefore if a different object was passed in for name than catalogName, that test would fail. However, even if name.equals(catalogName) was initially true in the old code it could proceed to do the search (possibly unnecessarily in the case where it would simply return the cached bundle anyway), but even if the search failed it would discover the String value of the name was equivalent to the String value of catalogName and then and only then return the cached catalog. Now, we simply check for matching String values at the beginning and return the cached catalog if the Locale hasn't changed. so the previously cached bundle was returned (this is frankly an odd case, because it also means that re-searching for a previously found bundle is now failing, which only seems possible if the bundle is/was (a) not available via the system classloader, (b) the context classloader, or (c), the classloaders up the call chain. The new behavior /does /still allow for a change in the default locale. For example, if you first call findResourceBundleName(foo) and the default locale is US, it will search for the foo_US bundle and set catalogLocale=US, and catalogName=foo and return the foo_US bundle. If you then search for foo and have changed the default locale to DE (if that is indeed a valid locale), it will then search for foo_DE and if found set catalogLocale=DE and cache and return the foo_DE bundle. The /only /change in behavior that I see here, is that if you change the locale and the bundle is not found, null will be returned instead of the previously cached bundle (if one exists), for a /different/ locale. Right. The behavioral difference that I point out was when foo_DE is cached, later the current locale is set to JP where foo_JP and foo don't exist, the old behavior returns foo_DE which was a bug too me whereas the new behavior returns null which matches the spec of Logger.getResourceBundle(). So, the old behavior in effect had a notion of a default catalog, but only if you happened to find a bundle on a previous lookup and cache it. However, you wouldn't be guaranteed of acquiring this same bundle on a subsequent search if a search for a different name had intervened. Thus, the new behavior is that if you search for a name/locale and it's not found, you will get null (every time). This seems better to me because it's consistent, explainable and simple. This matches what Logger.getResourceBundle() is specified and the behavior is correct. The difference in behavior is a minimal compatibility risk. Comments on the updated webrev: http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ L122-125: you may want to replace code/code with {@code } I think we'll save that for a later day - and then update all the occurrences of code to {@code} L129-130: you can use @linkplain instead: {@linkplain ClassLoader#getSystemClassLoader()system ClassLoader} All other existing links are using @link. Why should we use @linkplain here? L141-142: the bundle
Re: CFR - updated 8001667: Comparator combinators and extension methods
On 03/06/2013 03:28 AM, Ali Ebrahimi wrote: Hi, just one suggestion: rename comparing with compareWith There was a round of discussion on naming. http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2012-November/000446.html I have my personal preference among proposals, but EG seems to have come to a consensus on this. I don't feel strongly between thenCompare, thenComparing or thenCompareWith. But we should be consistent between Comparators and Comparator, and consider that Comparators methods could be static interface method on Comparator in the future. Cheers, Henry 1) public static T, U extends Comparable? super U ComparatorT compareWith(Function? super T, ? extends U keyExtractor) { 2) default ComparatorT thenCompareWith(Comparator? super T other) Best Regards, Ali Ebrahimi On Wed, Mar 6, 2013 at 12:16 AM, Henry Jen henry@oracle.com mailto:henry@oracle.com wrote: Hi, Another update to reflect functional interface renames involved in the API, and a bug fix for a regression found earlier. CCC had been approved. Can we get it reviewed and pushed? [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev Cheers, Henry
Re: Request for review- RFE 8005716
Actually for windows I *did* export the undecorated name. I just didn't see where I did it in the VS IDE. If you don't export the undecorated name it doesn't work of course. bill On 3/5/2013 11: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_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). I'll remove that. I redid the call to findBuiltinJniFunction but forgot to remove that. Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() an exported function in the VM? How does it find JNI_OnLoad_L? Via a table setup by the developer/build system when the library is linked in? bill dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Java regex vs. Unicode TR#18 vs. ICU
Hello, Someone on the ICU team recently compared the use of \w between ICU, Java, and Unicode TR#18 http://www.unicode.org/reports/tr18/#Compatibility_Properties . The results are in the following ICU bug http://bugs.icu-project.org/trac/ticket/10006. A question for core-libs-dev is, does Java plan to change the semantics of \w to match TR#18's list? Thanks, Steven
Re: CFR - updated 8001667: Comparator combinators and extension methods
On 03/06/2013 02:31 AM, Michael Hixson wrote: Hello, I'm not one of the people that you're looking at to review this, but I have to give this feedback anyway. I tried to give similar feedback on another mailing list and got no response (maybe I chose the wrong list). These changes have been bothering me. :) I find your earlier posts on lambda-libs-spec-comments archive. I was not on that list. 1. Why disable the following code even though it is (theoretically) safe? ComparatorCharSequence a = comparing(CharSequence::length); ComparatorString b = a.thenComparing(CASE_INSENSITIVE_ORDER); That code would compile if the signatures of the thenComparing methods had generics like S extends T instead of T. The example above is conjured up and ridiculous, but I think real code will have use for it. Is there any downside to narrowing the return type this way? I think that make sense, will need to do some experiment to make sure it won't confuse type system when chaining all together, and I am not sure how much burden this has on inference engine. I prefer simplicity. Theoretically, if all function take Comparator carefully allows super type, which we have, isn't that enough? Your above example can be, ComparatorString a = comparingCharSequence::length); a.thenComparing(CASE_INSENSITIVE_ORDER); 2. There's a thenComparing(Function), but no thenComparing(Function, Comparator). This seems like a big omission. Surely people will want secondary orderings on fields by something other than natural (null-unfriendly) ordering. Also, a Comparators.comparing(Function, Comparator) method would remove the need for all the Map-centric methods that are currently in the Comparators class. For instance: comparing(Map.Entry::getValue, naturalOrder()). Note that Function form must return a Comparable, which implies an order already. Combine with Comparator.reverseOrder() method, that would cover the ground. If the Comparable is not a fit, probably write a Comparator in lambda is needed anyway? 3. There is no support for sorting of nullable values or fields. Sorting of nullable values directly perhaps should not be encouraged, but sorting values by nullable fields within a chained sort is completely reasonable. Please add some support for this, either as chain methods on Comparator, or as factory methods on Comparators. Not sure what do you propose to be added. NULL is a controversial topic, only application know what it means. Therefore, avoid try to interpret null is probably a better approach. Write a Comparator if needed. 4. If Comparator had min(a,b) and max(a,b) methods, the Comparators.lesserOf(c) and greaterOf(c) methods would be unnecessary. The min/max methods would be generally more useful than the BinaryOperators returned from Comparators, and c::min reads better than Comparators.lesserOf(c). +1. 5. Comparators.reverseOrder(c) is confusing in that it has different behavior than Collections.reverseOrder(c). It throws an NPE. Also, this new method seems to be useless because one could call c.reverseOrder() instead. I suggest removing the method. I agree. 6. I don't see why Comparators.compose(c1,c2) is useful given that users can call c1.thenComparing(c2). The latter reads better; the word compose does not naturally fit with comparators and has strange connotations for those with Math backgrounds. 7. Last I checked, even this simple example did not compile: ComparatorString c = comparing(String::length); It was confused about whether I was implying a ToDoubleFunction or a ToLongFunction, which were both wrong. I also ran into a lot of type inference loop problems when chaining. Is this simply a problem with lambda evaluation that's going to be fixed before Java 8 is officially released? Is there something more complex going on here that makes statements like this impossible? If the compilation problems aren't going to be fixed prior to the release, or if they cannot be fixed, then none of these Comparator/Comparators additions are useful. You would be better off removing them. My hope is this to be fixed. Cheers, Henry
Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use
On 3/6/2013 11:42 AM, Jim Gish wrote: http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ L122-125: you may want to replace code/code with {@code } I think we'll save that for a later day - and then update all the occurrences of code to {@code} Fine with me. L129-130: you can use @linkplain instead: {@linkplain ClassLoader#getSystemClassLoader()system ClassLoader} All other existing links are using @link. Why should we use @linkplain here? The javadoc generated in your current change will show: system ClassLoader, a href=ClassLoader.getSystemClassLoader()/a, If you use @linkplain, the javadoc will show: a href=system ClassLoader/a LoggerResourceBundleRace.java: I think what you really want is to add a new test that sets a context class loader to a class loader that finds the resource bundle for a logger that a system class loader can't find. I suggest to leave this test as it is and then add a new test to exercise the context class loader search of a resource bundle as a separate RFE that will improve the test coverage. Leaving the existing test as is not an option unless we change the test to run in othervm as I had on my first webrev. The bundles are not found otherwise. Hence the change to set the context classloader. I'm confused. Is the resource bundle LoggerResourceBundleRace$MyResources in the classpath as this test class is located? Should the system class loader be able to find them? This test shouldn't depend on the stack search. Shouldn't the thread context classloader be null and then use the system class loader? Is another test set the thread context classloader in the jtreg run? Does the test pass if you run it directly (not via jtreg)? Mandy
Re: Java regex vs. Unicode TR#18 vs. ICU
On 03/06/2013 12:44 PM, Steven R. Loomis wrote: Hello, Someone on the ICU team recently compared the use of \w between ICU, Java, and Unicode TR#18 http://www.unicode.org/reports/tr18/#Compatibility_Properties . The results are in the following ICU bug http://bugs.icu-project.org/trac/ticket/10006. A question for core-libs-dev is, does Java plan to change the semantics of \w to match TR#18's list? It appears the standard has just added one more entry \p{Join_Control} during their last update :-( I may consider to update the spec/impl to match that, I would assume there is no any jdk7 application really has dependency on the updated \w (in jdk7). -Sherman
Re: Java regex vs. Unicode TR#18 vs. ICU
On 3/6/13 1:06 PM, Xueming Shen wrote: On 03/06/2013 12:44 PM, Steven R. Loomis wrote: Hello, Someone on the ICU team recently compared the use of \w between ICU, Java, and Unicode TR#18 http://www.unicode.org/reports/tr18/#Compatibility_Properties . The results are in the following ICU bug http://bugs.icu-project.org/trac/ticket/10006. A question for core-libs-dev is, does Java plan to change the semantics of \w to match TR#18's list? It appears the standard has just added one more entry \p{Join_Control} during their last update :-( I may consider to update the spec/impl to match that, I would assume there is no any jdk7 application really has dependency on the updated \w (in jdk7). -Sherman Thanks, Sherman Do you want to open a bug to track this? You can reference the above URLs Steven
Re: Request for review- RFE 8005716
On 3/6/2013 12:50 PM, Mike Duigou wrote: Hi Bill; Some notes from reviewing the JDK side changes. System.java/Runtime.java: The example which begins with the name If the filename argument, needs to better identify that L is an example. (Italics?) Re-worded that a bit. java/lang/ClassLoader.java: NativeLibrary::fromClass could be final. Ok. ClassLoader.c: In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib() These two values are known at compile time. int prefixLen = (int) strlen(JNI_LIB_PREFIX); int suffixLen = (int) strlen(JNI_LIB_SUFFIX); Some of the error conditions don't throw exceptions. Such as: if (cname == NULL) { return NULL; } Fixed. The prefix and suffix are stripped from cname without checking that cname actually contains the prefix or suffix. ..._findBuiltinLib is only called after System.mapLibraryName has added prefix and suffix. if (len prefixLen) is invariant. Based on System.mapLibraryName always called, then this could just be an assert. Unless JNU_GetStringPlatformChars returns some bogus string. src/solaris/native/common/jni_util_md.c: void* getProcessHandle() { static void* procHandle = NULL; if (procHandle == NULL) { procHandle = (void*)dlopen(NULL, RTLD_LAZY); } return procHandle; } Why is the error handling code commented out? That was just for some debugging. I've removed it. Thanks for commenting. I'll get a new webrev up shortly. bill Mike On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
hg: jdk8/tl/jdk: 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Changeset: 14e49a70729a Author:martin Date: 2013-03-06 17:43 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/14e49a70729a 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so Summary: Define FILES_m to force use of linker script Reviewed-by: sherman, alanb, ohair ! make/java/zip/Makefile ! src/share/native/java/util/zip/Inflater.c
Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Pushed to jdk8/tl/jdk. I recommend this be backported to jdk7u.
hg: jdk8/tl/jdk: 8009604: old make images failed: JarBASE64Encoder class not found
Changeset: cf54f6be3e9e Author:weijun Date: 2013-03-07 11:32 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf54f6be3e9e 8009604: old make images failed: JarBASE64Encoder class not found Reviewed-by: xuelei, wetmore ! make/common/Release.gmk