Re: RFR (JAXP): 8036004 : Incorrect license header for a test class
On 28/02/2014 07:51, huizhe wang wrote: Hi, This is a fix to the license header for class jdk/test/javax/xml/jaxp/transform/8004476/TestBase.java. Bug: https://bugs.openjdk.java.net/browse/JDK-8036004 Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8036004/webrev/ Looks okay, just check the date from the hg logs as you want to put 2013 in the new header. -Alan
JDK-8036003: Add variable not to separate debug information.
Hi all, Currently, configure script can accept --disable-debug-symbols and --disable-zip-debug-info as controlling to generate debug information. However, current makefiles cannot build ELF binaries which is contained debug information with images target. Some Linux distros use RPM as package manager. RPM is built by rpmbuild command, it strips symbols and debug information during to generate rpm packages. https://fedoraproject.org/wiki/Packaging:Debuginfo For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo . libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not contain debug information. Actual debug information is shipped by OpenJDK debuginfo package. This packaging is important when we have to debug JVM/JNI libraries. If we want to use debugging tools (GDB, SystemTap, etc...), they may requires debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is located at sub directory in /usr/lib/debug . It is defined in ELF section (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain valid debug information. We need to access libjvm.debuginfo.debug at same location. When we think to build OpenJDK rpm packages, we have to build ELF binaries which contain all debug information. we should not to separate debug information. Thus I want to add a variable SEPARATED_DEBUGINFO_FILES . If we pass SEPARATED_DEBUGINFO_FILES=false to make command, make does not execute objcopy command for generating debuginfo binaries. If we build rpm packages, we also have to add STRIP_POLICY=no_strip to arguments of make command. Or ELF binaries will be stripped. I've uploaded webrev for this enhancement. This is separated 3-part: top of forest, hotspot, jdk http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/ Please review it and sponsoring! Thanks, Yasumasa P.S. I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like --disable-separated-debug-info . I ran configure which is located at jdk9/dev directory after I ran autoconf and common/autoconf/autogen.sh. However it failed. I guess this is caused by changeset as below. JDK-8035495: Improvements in autoconf integration http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4 This changes add CHECKME option to configure script. However, this changes affects configure only. It should change configure.ac .
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
On Feb 27, 2014, at 3:51 PM, Dr Heinz M. Kabutz he...@javaspecialists.eu wrote: I have never used this in the wild, but rather have moved over to the ReentrantLock when I needed that particular functionality. However, I do see a place for this ability. As I wrote in here: http://www.javaspecialists.eu/archive/Issue194.html - sometimes you need to modify code that is already using a particular locking mechanism. To then redo everything with ReentrantLock can introduce errors. If I had a say, I would vote to either keep that method in Unsafe (which is where I think it belongs) or provide an alternative way to make the tryMonitorEnter() mechanism available to those that might end up needing it. Since I bought my Suzuki Jimny 7 years ago, I have not needed my spare wheel even once. So yeah, I could take it off and drive around without it. But chances are, the day after I take it off, I will need it. tryMonitorEnter() does no harm and it is out of reach of most programmers. I think your analogy to a spare wheel is misleading. Continuing with the car theme i liken this feature to a pair of fluffy dice dangling from the rear view mirror [1]: distracting; mostly useless; and mostly harmless. All the data so far indicates this is legacy code [2]. My suggestion, if you consider it important enough, is to write and circulate a draft JEP for such a public API [3]. Paul. [1] http://en.wikipedia.org/wiki/Fuzzy_dice [2] The only dependency on tryMonitorEnter i have found was very recently introduced in a test for which there are two known alternatives. [3] It would be interesting to know if some such public API was considered when j.u.c.locks were introduced, which was almost 10 years ago, and yet in all that time this feature is now virtually unused (not just by you but by many other Java developers too).
Re: RFR 9: 8035889: jdk testlibrary - add printing of values of failed assertions
Looks good to me! Thanks, /Staffan On 27 feb 2014, at 16:34, roger riggs roger.ri...@oracle.com wrote: Hi Mandy, I updated the webrev: http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ Alan suggested copying serviceability-dev so they have a chance to review if desired. I want to investigate if it is possible to use the TestNG Assert classes without the TestNG execution framework. It would be necessary to compile/run against TestNG.jar but it might not need the entire mechanism. Thanks, Roger On 2/26/2014 10:17 PM, Mandy Chung wrote: On 2/26/2014 7:09 PM, Roger Riggs wrote: Hi Mandy, Yes, it might be more productive to switch the tests to TestNG. But it did provide support in cases where TestNG could not be used, for example in a directory of existing tests that had custom reporting. But I remember there is a problem with TestNG having a dependency for XML which is not supported in Profile1 and a number of tests had to be disabled in that configuration. Will XML always be available. Do we need to solve or work around that problem with TestNG? This is a good point. When we want to test just the base module for example, how can we run TestNG tests? We need to address that certainly. My comment on TestNG is a question for new tests using this Asserts class. Your patch is fine to go (after taking out @library tag if I got it correct). Mandy Thanks, Roger On 2/26/14 9:01 PM, Mandy Chung wrote: Hi Roger, On 2/26/2014 12:34 PM, roger riggs wrote: The testlibrary for the jdk should be printing the values in the failed assertions to make debugging easier and quicker. The webrev adds the printing of the failed assertions and added methods for formatting and unconditional fail methods. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ AssertsTest.java: line 28: @library doesn't look like it's needed. There is no jdk/test/testlibrary directory and I think jdk.testlibrary.* are found as relative to $test.src. Otherwise, the change looks okay. Now that jtreg supports TestNG and I wonder if this class should retire some day (there are only about 10 existing tests using this class). Are you writing new tests using this Asserts class? Mandy Bug: 8035889: jdk testlibrary - add printing of values of failed assertions Thanks, Roger [1] https://bugs.openjdk.java.net/browse/JDK-8035889
Re: JDK-8036003: Add variable not to separate debug information.
Hi, As I put in the bug report this seems way too complicated. Seems to me all you need to do to get what you want is not use FDS and not strip the symbols from the binary. The former is trivial. The latter is more awkward as the strip policy stuff does not work as I would want it to work, but still doable. David On 28/02/2014 7:18 PM, Yasumasa Suenaga wrote: Hi all, Currently, configure script can accept --disable-debug-symbols and --disable-zip-debug-info as controlling to generate debug information. However, current makefiles cannot build ELF binaries which is contained debug information with images target. Some Linux distros use RPM as package manager. RPM is built by rpmbuild command, it strips symbols and debug information during to generate rpm packages. https://fedoraproject.org/wiki/Packaging:Debuginfo For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo . libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not contain debug information. Actual debug information is shipped by OpenJDK debuginfo package. This packaging is important when we have to debug JVM/JNI libraries. If we want to use debugging tools (GDB, SystemTap, etc...), they may requires debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is located at sub directory in /usr/lib/debug . It is defined in ELF section (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain valid debug information. We need to access libjvm.debuginfo.debug at same location. When we think to build OpenJDK rpm packages, we have to build ELF binaries which contain all debug information. we should not to separate debug information. Thus I want to add a variable SEPARATED_DEBUGINFO_FILES . If we pass SEPARATED_DEBUGINFO_FILES=false to make command, make does not execute objcopy command for generating debuginfo binaries. If we build rpm packages, we also have to add STRIP_POLICY=no_strip to arguments of make command. Or ELF binaries will be stripped. I've uploaded webrev for this enhancement. This is separated 3-part: top of forest, hotspot, jdk http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/ Please review it and sponsoring! Thanks, Yasumasa P.S. I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like --disable-separated-debug-info . I ran configure which is located at jdk9/dev directory after I ran autoconf and common/autoconf/autogen.sh. However it failed. I guess this is caused by changeset as below. JDK-8035495: Improvements in autoconf integration http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4 This changes add CHECKME option to configure script. However, this changes affects configure only. It should change configure.ac .
Re: JDK-8036003: Add variable not to separate debug information.
The proper way to fix this is to disable FDS. We should not need yet another option to control debug info. Dan On 2/28/14 4:13 AM, David Holmes wrote: Hi, As I put in the bug report this seems way too complicated. Seems to me all you need to do to get what you want is not use FDS and not strip the symbols from the binary. The former is trivial. The latter is more awkward as the strip policy stuff does not work as I would want it to work, but still doable. David On 28/02/2014 7:18 PM, Yasumasa Suenaga wrote: Hi all, Currently, configure script can accept --disable-debug-symbols and --disable-zip-debug-info as controlling to generate debug information. However, current makefiles cannot build ELF binaries which is contained debug information with images target. Some Linux distros use RPM as package manager. RPM is built by rpmbuild command, it strips symbols and debug information during to generate rpm packages. https://fedoraproject.org/wiki/Packaging:Debuginfo For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo . libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not contain debug information. Actual debug information is shipped by OpenJDK debuginfo package. This packaging is important when we have to debug JVM/JNI libraries. If we want to use debugging tools (GDB, SystemTap, etc...), they may requires debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is located at sub directory in /usr/lib/debug . It is defined in ELF section (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain valid debug information. We need to access libjvm.debuginfo.debug at same location. When we think to build OpenJDK rpm packages, we have to build ELF binaries which contain all debug information. we should not to separate debug information. Thus I want to add a variable SEPARATED_DEBUGINFO_FILES . If we pass SEPARATED_DEBUGINFO_FILES=false to make command, make does not execute objcopy command for generating debuginfo binaries. If we build rpm packages, we also have to add STRIP_POLICY=no_strip to arguments of make command. Or ELF binaries will be stripped. I've uploaded webrev for this enhancement. This is separated 3-part: top of forest, hotspot, jdk http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/ Please review it and sponsoring! Thanks, Yasumasa P.S. I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like --disable-separated-debug-info . I ran configure which is located at jdk9/dev directory after I ran autoconf and common/autoconf/autogen.sh. However it failed. I guess this is caused by changeset as below. JDK-8035495: Improvements in autoconf integration http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4 This changes add CHECKME option to configure script. However, this changes affects configure only. It should change configure.ac .
Re: [9] RFR (M): 8027827: Improve performance of catchException combinator
Thanks for review, John! Best regards, Vladimir Ivanov On 2/28/14 12:39 AM, John Rose wrote: On Feb 26, 2014, at 3:44 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com wrote: Maybe use invokeWithArguments with target and catcher? That at least is a one-liner, and probably more efficient. Yes, that's a good idea! At least, it considerably simplifies the code. Updated webrev: http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.03/ Thumbs up. Your use of invokeWithArguments in the unspecialized code is a good design pattern. The semantics are clear in the original method. This in turn gives a clear basis for specializing for each combination of argument arities and types. Specialization should be done using low-level, high-leverage mechanisms like bytecode spinning or even JIT optimizations. Put another way, if we have reasonable bytecode-generation intrinsics, feeding to good JIT optimizations, we don't need top-level specializations in the source code. The need for those has always been a mark of weakness in the HotSpot implementation of MHs. (Fredrik's JRockit implementation did it all in the JIT!) We will continue to push down specializations to lower layers. — John
Re: JDK-8036003: Add variable not to separate debug information.
The proper way to fix this is to disable FDS. Does this mean I have to pass --disable-debug-symbols to configure ? I've added comment to JDK-8036003, I think if we pass --disable-debug-symbols to configure, gcc/g++ is not passed -g option. -g is needed to generate debuginfo. Thanks, Yasumasa On 2014/02/28 23:04, Daniel D. Daugherty wrote: The proper way to fix this is to disable FDS. We should not need yet another option to control debug info. Dan On 2/28/14 4:13 AM, David Holmes wrote: Hi, As I put in the bug report this seems way too complicated. Seems to me all you need to do to get what you want is not use FDS and not strip the symbols from the binary. The former is trivial. The latter is more awkward as the strip policy stuff does not work as I would want it to work, but still doable. David On 28/02/2014 7:18 PM, Yasumasa Suenaga wrote: Hi all, Currently, configure script can accept --disable-debug-symbols and --disable-zip-debug-info as controlling to generate debug information. However, current makefiles cannot build ELF binaries which is contained debug information with images target. Some Linux distros use RPM as package manager. RPM is built by rpmbuild command, it strips symbols and debug information during to generate rpm packages. https://fedoraproject.org/wiki/Packaging:Debuginfo For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo . libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not contain debug information. Actual debug information is shipped by OpenJDK debuginfo package. This packaging is important when we have to debug JVM/JNI libraries. If we want to use debugging tools (GDB, SystemTap, etc...), they may requires debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is located at sub directory in /usr/lib/debug . It is defined in ELF section (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain valid debug information. We need to access libjvm.debuginfo.debug at same location. When we think to build OpenJDK rpm packages, we have to build ELF binaries which contain all debug information. we should not to separate debug information. Thus I want to add a variable SEPARATED_DEBUGINFO_FILES . If we pass SEPARATED_DEBUGINFO_FILES=false to make command, make does not execute objcopy command for generating debuginfo binaries. If we build rpm packages, we also have to add STRIP_POLICY=no_strip to arguments of make command. Or ELF binaries will be stripped. I've uploaded webrev for this enhancement. This is separated 3-part: top of forest, hotspot, jdk http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/ Please review it and sponsoring! Thanks, Yasumasa P.S. I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like --disable-separated-debug-info . I ran configure which is located at jdk9/dev directory after I ran autoconf and common/autoconf/autogen.sh. However it failed. I guess this is caused by changeset as below. JDK-8035495: Improvements in autoconf integration http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4 This changes add CHECKME option to configure script. However, this changes affects configure only. It should change configure.ac .
Re: RFR (JAXP): 8036004 : Incorrect license header for a test class
On 2/28/2014 12:01 AM, Alan Bateman wrote: On 28/02/2014 07:51, huizhe wang wrote: Hi, This is a fix to the license header for class jdk/test/javax/xml/jaxp/transform/8004476/TestBase.java. Bug: https://bugs.openjdk.java.net/browse/JDK-8036004 Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8036004/webrev/ Looks okay, just check the date from the hg logs as you want to put 2013 in the new header. Thanks, fixed (2013). -Joe -Alan
Re: rationale for swallowing exceptions in Class#getEnumConstantsShared
Am 28.02.2014 17:36, schrieb Peter Levart: [...] The other use of getEnumConstantsShared() is to support the Enum.valueOf(Class, String) method which already throws IllegalArgumentException if the 1st parameter does not represent an enum class, so there's no compatibility concerns about that use. Enum.valueOf is the method through which I discovered this. I have exactly this case, an enum, which seems not to fit getEnumConstantsShared for an unknown reason. My additional problem though is, that sometimes the call works and sometimes not. bye Jochen -- Jochen blackdrag Theodorou - Groovy Project Tech Lead blog: http://blackdragsview.blogspot.com/ german groovy discussion newsgroup: de.comp.lang.misc For Groovy programming sources visit http://groovy-lang.org
Re: rationale for swallowing exceptions in Class#getEnumConstantsShared
On 02/28/2014 04:56 PM, Jochen Theodorou wrote:
Hi,
we stumbled here about a strange error involving enums generated from
our compiler and that did make me see getEnumConstantsShared() in
java.lang.Class.
Now the implementation more or less says, that if calling values()
fails in any way, return null and ignore the exception. But why is the
exception ignored? It could contain valuable information about what
did go wrong
Hi Jochen,
I agree that the exception should be thrown (possibly an unchecked
IllegalArgumentException or something like that with the cause from
reflective operation). If Class.isEnum() returns true then the class
should be considered an enum and if it is not conformant to
specification (which may cause one of those exceptions) then this should
be reported as exception and not as null return. The
getEnumConstantsShared() is also used as implementation for public method:
/**
* Returns the elements of this enum class or *null if this**
** * Class object does not represent an enum type*.
*
* @return an array containing the values comprising the enum class
* represented by this Class object in the order they're
* declared, or null if this Class object does not
* represent an enum type
* @since 1.5
*/
public T[] getEnumConstants() {
T[] values = getEnumConstantsShared();
return (values != null) ? values.clone() : null;
}
The behaviour of above method is not consistent with:
/**
* Returns *true if and only if this class was declared as an enum*
in the
* source code.
*
* @return true if and only if this class was declared as an enum
in the
* source code
* @since 1.5
*/
public boolean isEnum() {
// An enum must both directly extend java.lang.Enum and have
// the ENUM bit set; classes for specialized enum constants
// don't do the former.
return (this.getModifiers() ENUM) != 0
this.getSuperclass() == java.lang.Enum.class;
}
...which can return true at the same time as getEnumConstants() returns
null for the same class...
I don't believe there's any code out there that relies on this faulty
behaviour of getEnumConstants() public method. The other use of
getEnumConstantsShared() is to support the Enum.valueOf(Class, String)
method which already throws IllegalArgumentException if the 1st
parameter does not represent an enum class, so there's no compatibility
concerns about that use.
What do experts say?
Regards, Peter
In my code for example, something obviously goes wrong... sometimes...
depending on moon-phases or something. and I have not the slightest
idea what.
bye Jochen
Re: JDK 9 RFR of JDK-8035452: Fix serial lint warnings in core libs
On 2/27/14 12:11 PM, Joe Darcy wrote: I am trying hard to remain blissfully ignorant of any more low-level details of the serialization format; however, I might not be successful on that goal much longer ;-) I believe your latter statement is correct. :-) My preference in a case like this is to add the svuid if for no other reason that is is simple to explain and understand, even if it is not strictly required. In general, it does seem reasonable to add a svuid in cases where it's difficult or impractical to prove that the lack of a svuid causes no compatibility issues. There is a difference in character between a serializable class in Java SE (java.* and javax.*) and the jdk.Exported(true) types in the JDK and a serializable class that lives in sun.* or some other jdk.Exported(false) area. For that latter, the serialization contract has to be different, with fewer guarantees, just as the general usage contract for those types has fewer guarantees. I think this is analogous to putting non-serializable classes into collections; the collection itself is serializable, but it won't be anymore if you put non-serializable objects into it. If a user happens to have a direct or indirect reference to an object of a JDK implementation type, the compatibility contract is weaker than if an object with a public Java SE type were being dealt with. I'm not sure I buy this. Unfortunately, serialization differs from the general usage contract in that serialization exposes internals. Just like it can expose private (non-transient) fields, serialization can also can expose what might otherwise look like purely internal implementation types. The canonical example of how an application can get a reference to an apparently internal class is java.util.TimeZone. If an app calls TimeZone.getDefault(), it gets back an instance of sun.util.calendar.ZoneInfo without ever mentioning this class by name. Furthermore, TimeZone and ZoneInfo are serializable, so they can be serialized directly or as part of another serializable object graph, and an instance of s.u.c.ZoneInfo does appear in the serialized byte stream. If s.u.c.ZoneInfo were not to have a svuid, an apparently innocuous change to it would clearly cause application incompatibilities. As it happens, s.u.c.ZoneInfo does have a svuid. I also note in passing that TimeZone, an abstract class, also has a svuid. Finally, EnumSet doesn't need a serial version UID. It's serialized using a proxy class, so EnumSet never appears in a serialized byte stream. (Note, its readObject throws an exception unconditionally.) So it's probably safe to suppress its serialization warning. Yes, EnumSet was a bit tricky, it is serializable itself, but uses a proxy internally. (Effective Java, 2nd edition both recommends the proxy pattern and recommends adding a svuid to all serializable classes, but doesn't explicitly give guidance to this combination of features.) To avoid adding a long comment explaining the proxy pattern and why a svuid on EnumSet isn't really required, my preference would just be to add the svuid if it doesn't cause any harm. In this case I think it's possible by local reasoning (looking elsewhere in the EnumSet.java source code) to determine that EnumSet instances themselves are never actually serialized, because of the use of the serialization proxy pattern. If nothing else, adding a svuid here sets a bad example, so suppressing the warning is reasonable. I don't think a long comment is necessary. Probably something like the following is sufficient: @SuppressWarnings(serial) // no serialVersionUID because of serialization proxy s'marks
hg: jdk8/tl/jdk: 8035777: Consistent Lambda construction
Changeset: 183a8c520b4a Author:rfield Date: 2014-02-28 10:43 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/183a8c520b4a 8035777: Consistent Lambda construction Reviewed-by: ahgross, briangoetz, dlsmith ! src/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java ! src/share/classes/java/lang/invoke/TypeConvertingMethodAdapter.java + test/java/lang/invoke/lambda/LambdaReceiver.java + test/java/lang/invoke/lambda/LambdaReceiverBridge.java + test/java/lang/invoke/lambda/LambdaReceiver_anotherpkg/LambdaReceiver_A.java + test/java/lang/invoke/lambda/LambdaReturn.java + test/java/lang/invoke/lambda/MetafactoryArityTest.java + test/java/lang/invoke/lambda/MetafactoryParameterCastTest.java + test/java/lang/invoke/lambda/MetafactorySamReturnTest.java
Re: RFR 6835233 : Fedora 9 jdk regression test failed: java/lang/instrument/ParallelTransformerLoader.sh
On 2/28/14 9:27 AM, Stuart Marks wrote: I guess there is some risk of adding new intermittent failures, but tackling @ignore'd tests is important too. Right - the main risk is that we will see this test fail again at some point in the future. I'll be keeping an eye out for that. Thanks for the review, guys. -Brent
Re: Signature of MethodHandleInfo.reflectAs is not specific enough
On Feb 25, 2014, at 3:13 AM, Ali Ebrahimi ali.ebrahimi1...@gmail.com wrote: I know, this is too late, but I want to share my suggestion: public T extends AccessibleObjectAnnotatedElement T reflectAs(Class? super T expected, MethodHandles.Lookup lookup) Isn't this the same as public T extends AccessibleObject T reflectAs... ? I think we considered AccessibleObject but rejected it as not buying anything significant compared with Member which is an interface. Perhaps public T extends Member AnnotatedElement T reflectAs... with both interfaces, would have been slightly better. As the API is written (and yes it is too late to change) I don't think there are any use cases (at least with ground types) which require an extra cast. Thank you for looking at it. — John
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
On Feb 28, 2014, at 1:38 AM, Paul Sandoz paul.san...@oracle.com wrote: But chances are, the day after I take it off, I will need it. tryMonitorEnter() does no harm and it is out of reach of most programmers. I think your analogy to a spare wheel is misleading. Continuing with the car theme i liken this feature to a pair of fluffy dice dangling from the rear view mirror [1]: distracting; mostly useless; and mostly harmless. The dice in your analogy are *too* useless; nobody can prove tryLock or isLocked useless, just in bad taste. It's more like a spare tire, maybe just for snow, which is really hard to access, as in break glass in case of emergency. I threw it in, way back when, knowing the operation is portable and natural, and expected that folks would break the glass if they needed it. Despite the barrier to entry, a couple folks did. All the data so far indicates this is legacy code [2]. I think the j.u.c library removed most needs to break the glass (connect to Unsafe). The users went to the standard API, and JVM native object monitors went out of fashion. But we are not ready to deprecate them, maybe never, so we should consider whether they need additional modernizations like tryLock or isLocked. My suggestion, if you consider it important enough, is to write and circulate a draft JEP for such a public API [3]. I second Paul's suggestion here. If you like it, buy the unlocked version! The cost includes working the JEP process, and persuading someone to engineer it. Since JMM++ is proposing some.field.volatile.readVolatile(), can someObject.synchronized.isLocked() be far behind? (OK, ugly bikeshed; System.getObjectMonitor(someObject).isLocked().) Part of the cost of working the JEP conversation will be convincing people that the proposed APIs cannot be subverted somehow or will somehow lead the unwary to write bad code. The Unsafe API sidesteps such conversations. Maybe that is its permanent niche: A locker to store those power tools which we can't figure out how to make safely usable by everybody. (Yes, I used permanent and sun.misc.Unsafe in a single thought, along with maybe. But let's try to wean ourselves away from it, at least bit by bit.) — John
RFR 9: 8035106 Typo in java.time.format.Parsed error message
Please review a typo correction in java.time.format.Parsed. The exception message is corrected. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-parsed-typo-8035106/ Thanks, Roger
RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec
Please review this bug fix from Steve Colebourne for java.time parsing of fixed width adjacent values. The patch includes new tests. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/ Thanks, Roger
Re: RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec
Looks Ok. Kind of surprised the tck tests have no assertion details in the tests. Minor nit would have been nice to have even a minor comment for the new method DateTimeFormatterBuilder though that seems to be the norm in some scenarios for the smaller methods. On Feb 28, 2014, at 4:48 PM, roger riggs wrote: Please review this bug fix from Steve Colebourne for java.time parsing of fixed width adjacent values. The patch includes new tests. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/ Thanks, Roger Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
RFR (JAXP): 8035469 : Xerces Update: EncodingMap does not recognise Java-style encodings Cp1141-Cp1149
Hi, This is an update from Xerces for a fixed encoding map entry in file EncodingMap.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035469 Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035469/webrev/ (I don't have a openjdk username yet, so Joe Wang uploaded it) No new tests since the change is minor. There were no tests from Apache fixes. Thanks, David
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
Except that Lock has features that are not supported by intrinsic locks
(timed wait, interruptible wait.) So the Lock returned would not
conform to Lock's contract, and attempting to call these methods would
probably throw UOE.
On 2/27/2014 6:12 AM, Stephen Colebourne wrote:
On 26 February 2014 20:54, Martin Buchholz marti...@google.com wrote:
It does seem that being able to tell whether a java object monitor is
currently locked is useful for debugging and monitoring - there should be a
way to do that.
Perhaps it would be useful to be able to expose a java object monitor
as an instance of Lock?
Lock lk = Lock.ofMonitor(object)
if (lk.tryLock()) {
...
}
Such a method feels like it would be a useful missing link between
synchronized and locks.
Stephen
Re: RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec
Hi Lance, I'll see if I can add some comments. The style for tests in java.time is less focused on statements in the spec due to the way the API developed. Perhaps the tests should have been just regular unit tests. Thanks, Roger On 2/28/2014 5:05 PM, Lance Andersen - Oracle wrote: Looks Ok. Kind of surprised the tck tests have no assertion details in the tests. Minor nit would have been nice to have even a minor comment for the new method DateTimeFormatterBuilder though that seems to be the norm in some scenarios for the smaller methods. On Feb 28, 2014, at 4:48 PM, roger riggs wrote: Please review this bug fix from Steve Colebourne for java.time parsing of fixed width adjacent values. The patch includes new tests. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/ Thanks, Roger
Re: RFR (JAXP): 8035469 : Xerces Update: EncodingMap does not recognise Java-style encodings Cp1141-Cp1149
Looks good. I'll sponsor the change. Thanks, Joe On 2/28/2014 2:16 PM, Lance Andersen - Oracle wrote: Looks fine. On Feb 28, 2014, at 5:11 PM, David Li wrote: Hi, This is an update from Xerces for a fixed encoding map entry in file EncodingMap.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035469 Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035469/webrev/ (I don't have a openjdk username yet, so Joe Wang uploaded it) No new tests since the change is minor. There were no tests from Apache fixes. 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
RFR 9: 8035813: Broken link in java.lang.Iterable
Please review this fix for a broken link in the javadoc. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-link-broken-8035813/ Thanks, Roger
Re: Unsafe: efficiently comparing two byte arrays
On 03/01/2014 02:29 AM, Martin Buchholz wrote: Are word-size reads in ByteBuffer actually intrinsified? I could find no evidence for that. Perhaps this is an important missing optimization for hotspot to make? It is a missing optimization, and we will get to that: https://bugs.openjdk.java.net/browse/JDK-8026049 -Aleksey.
Re: Unsafe: efficiently comparing two byte arrays
On 02/28/2014 10:29 PM, Martin Buchholz wrote: Are word-size reads in ByteBuffer actually intrinsified? No. Andrew.
Re: JDK-8036003: Add variable not to separate debug information.
On 1/03/2014 1:38 AM, Yasumasa Suenaga wrote: The proper way to fix this is to disable FDS. Does this mean I have to pass --disable-debug-symbols to configure ? I've added comment to JDK-8036003, I think if we pass --disable-debug-symbols to configure, gcc/g++ is not passed -g option. -g is needed to generate debuginfo. There are three pieces to all of this: 1. Generating debug symbols in the binaries (via gcc -g or whatever) 2. Generating debuginfo files (zipped or not) (FDS) 3. Stripping debug symbols from the binaries (strip-policy) It may be that we don't have clean separation between them, and if so that should be fixed, but I don't see the current proposal as the way forward. Also there may well be differences between how things are handled on the JDK side and hotspot side. I will try to look closer if I get a chance but my time is limited at the moment. David Thanks, Yasumasa On 2014/02/28 23:04, Daniel D. Daugherty wrote: The proper way to fix this is to disable FDS. We should not need yet another option to control debug info. Dan On 2/28/14 4:13 AM, David Holmes wrote: Hi, As I put in the bug report this seems way too complicated. Seems to me all you need to do to get what you want is not use FDS and not strip the symbols from the binary. The former is trivial. The latter is more awkward as the strip policy stuff does not work as I would want it to work, but still doable. David On 28/02/2014 7:18 PM, Yasumasa Suenaga wrote: Hi all, Currently, configure script can accept --disable-debug-symbols and --disable-zip-debug-info as controlling to generate debug information. However, current makefiles cannot build ELF binaries which is contained debug information with images target. Some Linux distros use RPM as package manager. RPM is built by rpmbuild command, it strips symbols and debug information during to generate rpm packages. https://fedoraproject.org/wiki/Packaging:Debuginfo For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo . libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not contain debug information. Actual debug information is shipped by OpenJDK debuginfo package. This packaging is important when we have to debug JVM/JNI libraries. If we want to use debugging tools (GDB, SystemTap, etc...), they may requires debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is located at sub directory in /usr/lib/debug . It is defined in ELF section (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain valid debug information. We need to access libjvm.debuginfo.debug at same location. When we think to build OpenJDK rpm packages, we have to build ELF binaries which contain all debug information. we should not to separate debug information. Thus I want to add a variable SEPARATED_DEBUGINFO_FILES . If we pass SEPARATED_DEBUGINFO_FILES=false to make command, make does not execute objcopy command for generating debuginfo binaries. If we build rpm packages, we also have to add STRIP_POLICY=no_strip to arguments of make command. Or ELF binaries will be stripped. I've uploaded webrev for this enhancement. This is separated 3-part: top of forest, hotspot, jdk http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/ Please review it and sponsoring! Thanks, Yasumasa P.S. I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like --disable-separated-debug-info . I ran configure which is located at jdk9/dev directory after I ran autoconf and common/autoconf/autogen.sh. However it failed. I guess this is caused by changeset as below. JDK-8035495: Improvements in autoconf integration http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4 This changes add CHECKME option to configure script. However, this changes affects configure only. It should change configure.ac .
Re: [REFRESH] JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger
On 2/27/14 12:14 PM, Brian Burkhalter wrote: On Feb 27, 2014, at 12:17 AM, Paul Sandoz wrote: So may I obtain a +1 from a JDK 9 Reviewer now? Indeed you may. Thanks, Paul. I refreshed the webrev http://cr.openjdk.java.net/~bpb/8035279/webrev.01/ with the agreed upon version. Hi Brian, This is pretty good. After this long, strange trip through the JMM, restoring the sentinel values to zeroes and renaming the fields to be explicit about how they represent the actual values seems to be the best approach. Paul's suggestion about using the term stable value in comments is good too. I took a look at the serialization stuff. The actual serialized form hasn't changed, so there should be no compatibility here with previous versions. There are some things in the serialization doc that ought to be brought up to date, though. Note that the docs for serialPersistentFields, readObject, and writeObject appear in the javadoc output, in the Serialized Form page, even though these members are private! Per another of Paul's comments, the @serial tag should be removed from bitCountPlusOne, bitLengthPlusOne, and lowestSetBitPlusTwo, since these fields do not appear in the serialized representation. The fields bitCount, bitLength, and lowestSetBit appear in the serialized form only for backward compatibility and are otherwise ignored, so their @serialField entries should just say that instead of describing how they were formerly used. Also, firstNonzeroByteNum is missing a @serialField entry, and it should have the same description as the others. Typo at 4236-4237, it says be\ndefault instead of by\ndefault. The comment at lines 4242-4246 should simply be removed. The first and third sentences are redundant with other docs. The second sentence, The magnitude field is used as a temporary store for the byte array that is deserialized is incorrect, as there is no longer a 'magnitude' field; a local is used instead. The @serialData tag at line 4316 for writeObject is misused; this is really intended for *extra* serial data written by writeObject after the writeFields() or defaultWriteObject() call, which doesn't occur here. It might be worth being explicit in writeObject's doc comment about writing -1's and -2's as the values for bitCount, bitLength, lowestSetBit, and firstNonzeroByteNum for compatibility with older implementations, even though current implementations will ignore these values. Thanks, s'marks
Re: [REFRESH] JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger
Woops, I forgot a couple points. The @serial for the 'signum' field at line 130 should be removed. Note that even though this has the same name as the field that appears in the serialized form, the text from the @serialField tag for 'signum' that's part of the serialPersistentFields doc comment is the one that actually appears on the Serialized Form page. It might be worthwhile copying the more verbose description from lines 125-128 to the @serialField tag text (lines 4206-4207) since this describes the requirements on the serialized form more precisely. I hate redundancy in documentation, though. Note that the field names in the ObjectStreamField entries of the serialPersistentFields array don't necessarily match the actual fields in the class. In this case 'signum' matches but the others do not. That's ok. The entries in this array describe the field names that are used in the serialized output, which is essential for remaining compatible with older versions of BigInteger. I checked an old version of BigInteger from 1998 and the field names used here match the actual, serialized fields from that old version. s'marks On 2/28/14 5:35 PM, Stuart Marks wrote: On 2/27/14 12:14 PM, Brian Burkhalter wrote: http://cr.openjdk.java.net/~bpb/8035279/webrev.01/ with the agreed upon version. Hi Brian, This is pretty good. After this long, strange trip through the JMM, restoring the sentinel values to zeroes and renaming the fields to be explicit about how they represent the actual values seems to be the best approach. Paul's suggestion about using the term stable value in comments is good too. I took a look at the serialization stuff. The actual serialized form hasn't changed, so there should be no compatibility here with previous versions. There are some things in the serialization doc that ought to be brought up to date, though. Note that the docs for serialPersistentFields, readObject, and writeObject appear in the javadoc output, in the Serialized Form page, even though these members are private! Per another of Paul's comments, the @serial tag should be removed from bitCountPlusOne, bitLengthPlusOne, and lowestSetBitPlusTwo, since these fields do not appear in the serialized representation. The fields bitCount, bitLength, and lowestSetBit appear in the serialized form only for backward compatibility and are otherwise ignored, so their @serialField entries should just say that instead of describing how they were formerly used. Also, firstNonzeroByteNum is missing a @serialField entry, and it should have the same description as the others. Typo at 4236-4237, it says be\ndefault instead of by\ndefault. The comment at lines 4242-4246 should simply be removed. The first and third sentences are redundant with other docs. The second sentence, The magnitude field is used as a temporary store for the byte array that is deserialized is incorrect, as there is no longer a 'magnitude' field; a local is used instead. The @serialData tag at line 4316 for writeObject is misused; this is really intended for *extra* serial data written by writeObject after the writeFields() or defaultWriteObject() call, which doesn't occur here. It might be worth being explicit in writeObject's doc comment about writing -1's and -2's as the values for bitCount, bitLength, lowestSetBit, and firstNonzeroByteNum for compatibility with older implementations, even though current implementations will ignore these values. Thanks, s'marks
Re: [REFRESH] JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger
Hi Stuart On Feb 28, 2014, at 5:35 PM, Stuart Marks wrote: This is pretty good. After this long, strange trip through the JMM, restoring the sentinel values to zeroes and renaming the fields to be explicit about how they represent the actual values seems to be the best approach. Paul's suggestion about using the term stable value in comments is good too. I took a look at the serialization stuff. The actual serialized form hasn't changed, so there should be no compatibility here with previous versions. For the above, good! There are some things in the serialization doc that ought to be brought up to date, though. Note that the docs for serialPersistentFields, readObject, and writeObject appear in the javadoc output, in the Serialized Form page, even though these members are private! Per another of Paul's comments, the @serial tag should be removed from bitCountPlusOne, bitLengthPlusOne, and lowestSetBitPlusTwo, since these fields do not appear in the serialized representation. Yes, I was wondering about that. The fields bitCount, bitLength, and lowestSetBit appear in the serialized form only for backward compatibility and are otherwise ignored, so their @serialField entries should just say that instead of describing how they were formerly used. Also, firstNonzeroByteNum is missing a @serialField entry, and it should have the same description as the others. Typo at 4236-4237, it says be\ndefault instead of by\ndefault. The comment at lines 4242-4246 should simply be removed. The first and third sentences are redundant with other docs. The second sentence, The magnitude field is used as a temporary store for the byte array that is deserialized is incorrect, as there is no longer a 'magnitude' field; a local is used instead. The @serialData tag at line 4316 for writeObject is misused; this is really intended for *extra* serial data written by writeObject after the writeFields() or defaultWriteObject() call, which doesn't occur here. It might be worth being explicit in writeObject's doc comment about writing -1's and -2's as the values for bitCount, bitLength, lowestSetBit, and firstNonzeroByteNum for compatibility with older implementations, even though current implementations will ignore these values. I will make the suggested updates and repost an updated webrev, probably on Monday. Thanks for the detailed review! Brian
Re: [REFRESH] JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger
On Feb 28, 2014, at 5:54 PM, Stuart Marks wrote: Woops, I forgot a couple points. The @serial for the 'signum' field at line 130 should be removed. Note that even though this has the same name as the field that appears in the serialized form, the text from the @serialField tag for 'signum' that's part of the serialPersistentFields doc comment is the one that actually appears on the Serialized Form page. It might be worthwhile copying the more verbose description from lines 125-128 to the @serialField tag text (lines 4206-4207) since this describes the requirements on the serialized form more precisely. I hate redundancy in documentation, though. Note that the field names in the ObjectStreamField entries of the serialPersistentFields array don't necessarily match the actual fields in the class. In this case 'signum' matches but the others do not. That's ok. The entries in this array describe the field names that are used in the serialized output, which is essential for remaining compatible with older versions of BigInteger. That was my understanding so it's good to have it corroborated. I checked an old version of BigInteger from 1998 and the field names used here match the actual, serialized fields from that old version. This sort of detail is really good to hear. I'll incorporate the lot when I update. Thanks much! Brian
