RE: Thoughts on adding getElementClass() method to StackTraceElement?
Nick Williams wrote: What if we also added a getStackFrames() method to Throwable? That would meet my needs but it would also satisfy what I'm observing is a desire to have a new API for this (StackFrame) instead of adding it to StackTraceElement. I'm very open to how it's implemented, as long as it satisfies my use case. :-) The stack trace of a Throwable can be filled in on demand when getStackTrace() is called the first time, so that the overhead isn't incurred when creating and throwing the exception. Presumably, we would need to do something similar with getStackFrames(), especially since calling it would be less common. Thoughts on this? Yes that is reasonable, but I'd add a static method to StackFrame instead. Something like StackFrame[] capture(Throwable). Regards, Jeroen
Re: Thoughts on adding getElementClass() method to StackTraceElement?
On 06/17/2013 08:06 AM, Jeroen Frijters wrote: Nick Williams wrote: What if we also added a getStackFrames() method to Throwable? That would meet my needs but it would also satisfy what I'm observing is a desire to have a new API for this (StackFrame) instead of adding it to StackTraceElement. I'm very open to how it's implemented, as long as it satisfies my use case. :-) The stack trace of a Throwable can be filled in on demand when getStackTrace() is called the first time, so that the overhead isn't incurred when creating and throwing the exception. Presumably, we would need to do something similar with getStackFrames(), especially since calling it would be less common. Thoughts on this? Yes that is reasonable, but I'd add a static method to StackFrame instead. Something like StackFrame[] capture(Throwable). New API could be entirely unrelated to Throwable, if there was support for it in native code. Since there would have to be changes to the native code anyway to support this, why not create a separate API? Regards, Peter Regards, Jeroen
hg: jdk8/tl/nashorn: 4 new changesets
Changeset: e857ab684db0 Author:cl Date: 2013-06-06 20:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/e857ab684db0 Added tag jdk8-b93 for changeset ddbf41575a2b ! .hgtags Changeset: d92b756bc739 Author:lana Date: 2013-06-10 17:04 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/d92b756bc739 Merge - src/jdk/nashorn/internal/objects/DateParser.java Changeset: cbc9926f5b40 Author:katleman Date: 2013-06-13 09:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/cbc9926f5b40 Added tag jdk8-b94 for changeset d92b756bc739 ! .hgtags Changeset: 558d31c168ed Author:lana Date: 2013-06-16 22:38 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/558d31c168ed Merge
hg: jdk8/tl: 13 new changesets
Changeset: 33b6df33a2b7 Author:erikj Date: 2013-05-29 13:58 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/33b6df33a2b7 8013920: Configure sets JOBS to 0 if memory is too low. Reviewed-by: tbell ! common/autoconf/build-performance.m4 ! common/autoconf/generated-configure.sh Changeset: 03e60e87d92a Author:erikj Date: 2013-05-29 14:01 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/03e60e87d92a 8013489: New build system does not run codesign on SA-related launchers on OS X Reviewed-by: sla, tbell ! common/autoconf/basics.m4 ! common/autoconf/generated-configure.sh ! common/autoconf/spec.gmk.in ! common/makefiles/MakeBase.gmk ! common/makefiles/NativeCompilation.gmk Changeset: c31e9dc1fe3d Author:erikj Date: 2013-05-31 14:07 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/c31e9dc1fe3d 8014003: New build does not handle symlinks in workspace path Reviewed-by: tbell ! common/autoconf/basics.m4 ! common/autoconf/basics_windows.m4 ! common/autoconf/generated-configure.sh Changeset: 44259699e0b5 Author:erikj Date: 2013-06-04 10:23 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/44259699e0b5 8015784: Add configure parameter --with-update-version Reviewed-by: tbell, katleman, erikj Contributed-by: tristan@oracle.com ! common/autoconf/generated-configure.sh ! common/autoconf/jdk-options.m4 Changeset: db3144e1f89b Author:mduigou Date: 2013-06-04 10:36 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/db3144e1f89b 8015510: (s) Improve JTReg location detection and provide location to test/Makefile Reviewed-by: erikj ! common/autoconf/generated-configure.sh ! common/autoconf/toolchain.m4 ! common/makefiles/Main.gmk Changeset: 9b8e8098172c Author:katleman Date: 2013-06-04 11:02 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/9b8e8098172c Merge Changeset: f55734874c4f Author:katleman Date: 2013-06-04 15:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/f55734874c4f Merge ! common/autoconf/generated-configure.sh Changeset: 27c51c6e31c1 Author:katleman Date: 2013-06-05 15:20 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/27c51c6e31c1 6983966: remove lzma and upx from repository JDK8 Reviewed-by: tbell, paulk, ngthomas ! common/autoconf/generated-configure.sh ! common/makefiles/Jprt.gmk ! make/deploy-rules.gmk Changeset: 8dfb6ee04114 Author:katleman Date: 2013-06-06 09:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/8dfb6ee04114 Added tag jdk8-b93 for changeset 27c51c6e31c1 ! .hgtags Changeset: 198d25db45da Author:erikj Date: 2013-06-11 13:08 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/198d25db45da 8008707: build-infra: Closed (deploy) can't be built using environment from SDK SetEnv.cmd Reviewed-by: tbell ! common/autoconf/generated-configure.sh ! common/autoconf/toolchain_windows.m4 Changeset: 3cbcc2b6ba41 Author:erikj Date: 2013-06-11 13:25 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/3cbcc2b6ba41 8010785: JDK 8 build on Linux fails with new build mechanism Reviewed-by: dholmes, tbell ! common/autoconf/generated-configure.sh ! common/autoconf/jdk-options.m4 Changeset: 50d2bde060f2 Author:erikj Date: 2013-06-12 10:33 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/rev/50d2bde060f2 Merge Changeset: 6337f652e71f Author:katleman Date: 2013-06-13 09:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/6337f652e71f Added tag jdk8-b94 for changeset 50d2bde060f2 ! .hgtags
hg: jdk8/tl/jaxws: 3 new changesets
Changeset: 254c53fd97ab Author:katleman Date: 2013-06-06 09:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxws/rev/254c53fd97ab Added tag jdk8-b93 for changeset 7386eca865e1 ! .hgtags Changeset: 1468c94135f9 Author:katleman Date: 2013-06-13 09:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxws/rev/1468c94135f9 Added tag jdk8-b94 for changeset 254c53fd97ab ! .hgtags Changeset: c4191a46e3eb Author:lana Date: 2013-06-16 22:33 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxws/rev/c4191a46e3eb Merge
hg: jdk8/tl/jaxp: 4 new changesets
Changeset: 40da96cab40e Author:katleman Date: 2013-06-06 09:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/40da96cab40e Added tag jdk8-b93 for changeset d583a491d63c ! .hgtags Changeset: c84658e1740d Author:lana Date: 2013-06-10 16:59 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/c84658e1740d Merge Changeset: b8c5f4b6f0ff Author:katleman Date: 2013-06-13 09:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/b8c5f4b6f0ff Added tag jdk8-b94 for changeset c84658e1740d ! .hgtags Changeset: 0142ef23f1b4 Author:lana Date: 2013-06-16 22:32 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/0142ef23f1b4 Merge
hg: jdk8/tl/corba: 3 new changesets
Changeset: 22f5d7f261d9 Author:katleman Date: 2013-06-06 09:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/corba/rev/22f5d7f261d9 Added tag jdk8-b93 for changeset 8dc9d7ccbb2d ! .hgtags Changeset: 2cf36f43df36 Author:katleman Date: 2013-06-13 09:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/corba/rev/2cf36f43df36 Added tag jdk8-b94 for changeset 22f5d7f261d9 ! .hgtags Changeset: 0fac0a9d9545 Author:lana Date: 2013-06-16 22:30 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/corba/rev/0fac0a9d9545 Merge
Re: Thoughts on adding getElementClass() method to StackTraceElement?
On Jun 17, 2013, at 1:12 AM, Peter Levart wrote:
On 06/17/2013 08:06 AM, Jeroen Frijters wrote:
Nick Williams wrote:
What if we also added a getStackFrames() method to Throwable? That would
meet my needs but it would also satisfy what I'm observing is a desire
to have a new API for this (StackFrame) instead of adding it to
StackTraceElement. I'm very open to how it's implemented, as long as it
satisfies my use case. :-)
The stack trace of a Throwable can be filled in on demand when
getStackTrace() is called the first time, so that the overhead isn't
incurred when creating and throwing the exception. Presumably, we would
need to do something similar with getStackFrames(), especially since
calling it would be less common.
Thoughts on this?
Yes that is reasonable, but I'd add a static method to StackFrame instead.
Something like StackFrame[] capture(Throwable).
New API could be entirely unrelated to Throwable, if there was support for it
in native code. Since there would have to be changes to the native code
anyway to support this, why not create a separate API?
I'm not sure who's misunderstanding who. :-)
If some third party code that I have no control over throws an exception and I
catch that exception, or some other code catches the exception and passes it to
my code (because my code is a logging framework), I need to get the
StackFrame[] for _when the exception was thrown_. Not the StackFrame[] related
to my current method execution. How can that possibly be entirely unrelated to
throwable?
The way I understand Jereon's suggestion, I'm thinking StackFrame would look
like this:
public final class StackFrame {
public Executable method();
public String getFileName();
public int getLineNumber();
/** Shortcut for getModifiers() and Modifiers.NATIVE */
public int isNativeMethod();
/** Format exactly like StackTraceElement#toString() */
public String toString();
/** Gets current executing stack with number of frames skipped and max length
*/
public static StackFrame[] capture(int skipFrames, int maxLength, boolean
includeSourceInfo);
/** Gets current executing stack with no frames skipped and no max length */
public static StackFrame[] capture();
/** Gets stack from when Throwable was created. */
public static StackFrame[] capture(Throwable t);
}
But perhaps I am missing something.
Nick
hg: jdk8/tl/hotspot: 65 new changesets
Changeset: 61dcf187a198 Author:katleman Date: 2013-06-06 09:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/61dcf187a198 Added tag jdk8-b93 for changeset 573d86d412cd ! .hgtags Changeset: 194b27b865bc Author:amurillo Date: 2013-05-24 09:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/194b27b865bc 8015305: new hotspot build - hs25-b35 Reviewed-by: jcoomes ! make/hotspot_version Changeset: ccdecfece956 Author:bharadwaj Date: 2013-05-21 16:17 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/ccdecfece956 8014059: JSR292: Failed to reject invalid class cplmhl00201m28n Summary: Restrict reference of interface methods by invokestatic and invokespecial to classfile version 52 or later. Reviewed-by: kvn, hseigel ! src/share/vm/classfile/classFileParser.cpp Changeset: f54c85acc043 Author:mikael Date: 2013-05-21 09:43 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/f54c85acc043 8013726: runtime/memory/ReserveMemory.java fails due to 'assert(bytes % os::vm_allocation_granularity() == 0) failed: reserve block size' Summary: Fix regression test to work on all platforms Reviewed-by: ctornqvi, dholmes ! src/share/vm/prims/whitebox.cpp ! test/runtime/memory/ReserveMemory.java ! test/testlibrary/whitebox/sun/hotspot/WhiteBox.java Changeset: 1a07e086ff28 Author:dholmes Date: 2013-05-21 19:52 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/1a07e086ff28 Merge Changeset: 6bd680e9ea35 Author:coleenp Date: 2013-05-22 14:37 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/6bd680e9ea35 8003421: NPG: Move oops out of InstanceKlass into mirror Summary: Inject protection_domain, signers, init_lock into java_lang_Class Reviewed-by: stefank, dholmes, sla ! agent/src/share/classes/sun/jvm/hotspot/memory/DictionaryEntry.java ! agent/src/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java ! agent/src/share/classes/sun/jvm/hotspot/utilities/HeapGXLWriter.java ! agent/src/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java ! agent/src/share/classes/sun/jvm/hotspot/utilities/soql/JSJavaInstanceKlass.java ! src/share/vm/classfile/classFileParser.cpp ! src/share/vm/classfile/javaClasses.cpp ! src/share/vm/classfile/javaClasses.hpp ! src/share/vm/classfile/vmSymbols.hpp ! src/share/vm/oops/arrayKlass.cpp ! src/share/vm/oops/instanceKlass.cpp ! src/share/vm/oops/instanceKlass.hpp ! src/share/vm/oops/klass.cpp ! src/share/vm/oops/klass.hpp ! src/share/vm/oops/objArrayKlass.hpp ! src/share/vm/oops/typeArrayKlass.hpp ! src/share/vm/prims/jvm.cpp ! src/share/vm/runtime/vmStructs.cpp Changeset: 699d9df07e59 Author:ctornqvi Date: 2013-05-23 17:39 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/699d9df07e59 8009576: Test returns ClassNotFoundException Summary: Small classpath fix and move tests into open Reviewed-by: mgerdin, zgu + test/runtime/Metaspace/FragmentMetaspace.java + test/runtime/Metaspace/FragmentMetaspaceSimple.java + test/runtime/Metaspace/classes/test/Empty.java + test/runtime/testlibrary/GeneratedClassLoader.java Changeset: b7fa10a3a69a Author:sspitsyn Date: 2013-05-23 23:04 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/b7fa10a3a69a 8014288: perf regression in nashorn JDK-8008448.js test after 8008511 changes Summary: The fix of perf regression is to use method_idnum() for direct indexing into NMT Reviewed-by: twisti, kvn, coleenp, dholmes Contributed-by: serguei.spit...@oracle.com ! src/share/vm/oops/instanceKlass.cpp ! src/share/vm/oops/instanceKlass.hpp ! src/share/vm/prims/methodHandles.cpp ! src/share/vm/prims/methodHandles.hpp Changeset: cd83e1d98347 Author:dcubed Date: 2013-05-24 10:21 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/cd83e1d98347 Merge Changeset: 6c138b9851fb Author:sspitsyn Date: 2013-05-24 17:36 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/6c138b9851fb 8013945: CMS fatal error: must own lock MemberNameTable_lock Summary: The delete mnt needs to grab MemberNameTable_lock if !SafepointSynchronize::is_at_safepoint() Reviewed-by: sla, mgerdin, dholmes, jmasa Contributed-by: serguei.spit...@oracle.com ! src/share/vm/oops/instanceKlass.cpp Changeset: 3970971c91e0 Author:shade Date: 2013-05-27 12:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/3970971c91e0 8015270: @Contended: fix multiple issues in the layout code Summary: field count handling fixed, has_nonstatic_fields invariant fixed, oop map overrun fixed; new asserts Reviewed-by: kvn, dcubed, coleenp ! src/share/vm/classfile/classFileParser.cpp + test/runtime/contended/HasNonStatic.java + test/runtime/contended/OopMaps.java Changeset: a213d425d87a Author:ctornqvi Date: 2013-05-28 15:08 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/a213d425d87a 8015329: Print reason for failed MiniDumpWriteDump() call
hg: jdk8/tl/langtools: 4 new changesets
Changeset: 888386fddc09 Author:katleman Date: 2013-06-06 09:55 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/888386fddc09 Added tag jdk8-b93 for changeset 2c5a568ee36e ! .hgtags Changeset: 48c6e6ab7c81 Author:lana Date: 2013-06-10 17:04 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/48c6e6ab7c81 Merge Changeset: 4cb113623127 Author:katleman Date: 2013-06-13 09:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/4cb113623127 Added tag jdk8-b94 for changeset 48c6e6ab7c81 ! .hgtags Changeset: 1eb09dba594a Author:lana Date: 2013-06-16 22:38 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/1eb09dba594a Merge
Re: Thoughts on adding getElementClass() method to StackTraceElement?
On 06/15/2013 09:59 PM, Nick Williams wrote: On Jun 15, 2013, at 2:22 PM, Remi Forax wrote: Could you be a little more clear why you need a class for your logging API, exactly, why the class name is not enough for logging purpose ? Certainly. Log4j 2 offers extended stack trace patterns. When exceptions or stack traces are logged, or when displaying the location that the logging statement originated from, the stack trace/log also reveals the resource (JAR file, directory, etc.) that the class was loaded from and the implementation version from the package the class is in. This helps reveal possible class loading issues (among other things), which can be an extremely valuable tool when troubleshooting problems with an application. Log4j 2 accomplishes this by getting the class associated with a StackTraceElement (as outlined below), and then 1) getting the Package from the Class and the version from the Package, and 2) getting the ProtectionDomain from the Class, getting the CodeSource from the ProtectionDomain, and getting the JAR/URL from the CodeSource. The last two parts are easy. It's getting the Class from the StackTrace that is extremely inefficient if not done right. Java 8 took away the best way we had found to do that (which, admittedly, was because we were relying on a non-standard class, which isn't recommended). On 06/17/2013 08:12 AM, Peter Levart wrote: New API could be entirely unrelated to Throwable, if there was support for it in native code. Since there would have to be changes to the native code anyway to support this, why not create a separate API? If I understand this correctly, one part of your code is displaying the resource location of each method's declaring class in the stack trace when such extended stack trace is configured. Therefore you need this API to capture the class-enriched-stack-trace at the time the Throwable instance is constructed. No special Throwable-unrelated API to return the stack trace would be suitable for this purpose then. But for the purpose of displaying the resource location of the class invoking the logging API, you don't need the entire stack trace, just the immediate caller class, right? When @CallerSensitive internal annotation was introduced (and Reflection.getCallerClass(int frames) was dropped), there was some debate about possible introduction of public API that could be used to reveal the caller's class. Perhaps there's still time to create such API for JDK8? At least one part of your logging functionality would be covered with such API. Suppose the API to obtain the immediate caller class (j.l.Class object) was public and fast, so you could do that for each call to the logging API eagerly (after initial check for logging level). Now, could you delay the evaluation of extended stack trace pattern to the latest time possible, when you must format the stack-trace? Would calling Class.forName(name, false, classLoader) for each individual StackTraceElement, using the ClassLoader of the caller class, still present a performance problem? You would only get locations for classes that the caller has access to, so no inaccessible information could be revealed. Regards, Peter
hg: jdk8/tl/jdk: 36 new changesets
Changeset: 583e6dec1ed7 Author:erikj Date: 2013-05-29 14:01 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/583e6dec1ed7 8013489: New build system does not run codesign on SA-related launchers on OS X Reviewed-by: sla, tbell ! makefiles/CompileLaunchers.gmk Changeset: d8c97d6772cd Author:erikj Date: 2013-05-30 09:29 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d8c97d6772cd Merge Changeset: bc3a17982aae Author:erikj Date: 2013-05-31 14:05 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bc3a17982aae 7195481: FDS: debuginfo file for libjdwp.so is missed Reviewed-by: tbell ! make/jpda/back/Makefile ! makefiles/CompileNativeLibraries.gmk Changeset: c50add191a39 Author:katleman Date: 2013-06-04 11:03 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c50add191a39 Merge ! makefiles/CompileNativeLibraries.gmk Changeset: 16003f414ca3 Author:katleman Date: 2013-06-04 14:11 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/16003f414ca3 8015644: makefile changes to allow integration of new features Reviewed-by: tbell, erikj, dholmes Contributed-by: amy.y.w...@oracle.com ! makefiles/Images.gmk Changeset: 691d6c6cd332 Author:katleman Date: 2013-06-05 15:25 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/691d6c6cd332 6983966: remove lzma and upx from repository JDK8 Reviewed-by: tbell, paulk, ngthomas ! make/common/Defs-windows.gmk Changeset: 7b757d567346 Author:katleman Date: 2013-06-06 09:55 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7b757d567346 Added tag jdk8-b93 for changeset 691d6c6cd332 ! .hgtags Changeset: fd377533608b Author:andrew Date: 2013-05-30 16:50 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fd377533608b 8011693: Remove redundant fontconfig files Summary: Remove unused fontconfig files from OpenJDK GNU/Linux builds Reviewed-by: andrew, prr Contributed-by: Jiri Vanek jva...@redhat.com ! make/sun/awt/Makefile ! makefiles/GendataFontConfig.gmk - src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.Fedora.properties - src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.SuSE.properties - src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.Ubuntu.properties - src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.properties Changeset: b9b73bf450a4 Author:bae Date: 2013-05-31 14:30 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b9b73bf450a4 8015606: Text is not rendered correctly if destination buffer is custom Reviewed-by: prr, vadim ! src/share/classes/sun/java2d/loops/MaskFill.java + test/sun/java2d/loops/RenderToCustomBufferTest.java Changeset: 0a17344d074e Author:prr Date: 2013-05-31 09:25 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0a17344d074e 8015556: [macosx] surrogate pairs do not render properly. Reviewed-by: bae, jchen ! src/macosx/classes/sun/font/CCharToGlyphMapper.java + test/java/awt/FontClass/SurrogateTest/SuppCharTest.java Changeset: 3af3981dee11 Author:lana Date: 2013-06-05 09:52 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3af3981dee11 Merge - test/com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.sh Changeset: 768fcc36182a Author:anthony Date: 2013-05-30 18:10 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/768fcc36182a 8015303: [macosx] Application launched via custom URL Scheme does not receive URL Summary: Make copies of event parameters Reviewed-by: anthony, swingler, serb Contributed-by: James Tomson james.b.tom...@gmail.com ! src/macosx/native/sun/osxapp/QueuingApplicationDelegate.m Changeset: 8472c148688c Author:ant Date: 2013-05-30 18:23 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8472c148688c 8013424: Regression: java.awt.datatransfer.FlavorListeners not notified on Linux/Java 7 Reviewed-by: anthony ! src/solaris/classes/sun/awt/X11/XClipboard.java Changeset: 56512cfccef9 Author:ant Date: 2013-05-30 18:31 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/56512cfccef9 8013773: requestFocusInWindow to a disabled component prevents window of getting focused Reviewed-by: leonidr, alexsch ! src/share/classes/java/awt/DefaultKeyboardFocusManager.java + test/java/awt/Focus/ResetMostRecentFocusOwnerTest/ResetMostRecentFocusOwnerTest.java Changeset: b0eab0f8b503 Author:anthony Date: 2013-05-31 14:12 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b0eab0f8b503 8013189: JMenuItems draw behind TextArea Summary: Untie XTextAreaPeer internal components from the TextArea parent to prevent its invalidation. I.e. force the java.awt.smartInvalidate=true locally. Reviewed-by: art, serb ! src/solaris/classes/sun/awt/X11/XTextAreaPeer.java + test/java/awt/TextArea/Mixing/TextAreaMixing.java Changeset: 481476e941fd Author:ant Date: 2013-05-31 15:56 +0400 URL:
Re: Thoughts on adding getElementClass() method to StackTraceElement?
On Jun 17, 2013, at 1:55 AM, Peter Levart wrote: On 06/15/2013 09:59 PM, Nick Williams wrote: On Jun 15, 2013, at 2:22 PM, Remi Forax wrote: Could you be a little more clear why you need a class for your logging API, exactly, why the class name is not enough for logging purpose ? Certainly. Log4j 2 offers extended stack trace patterns. When exceptions or stack traces are logged, or when displaying the location that the logging statement originated from, the stack trace/log also reveals the resource (JAR file, directory, etc.) that the class was loaded from and the implementation version from the package the class is in. This helps reveal possible class loading issues (among other things), which can be an extremely valuable tool when troubleshooting problems with an application. Log4j 2 accomplishes this by getting the class associated with a StackTraceElement (as outlined below), and then 1) getting the Package from the Class and the version from the Package, and 2) getting the ProtectionDomain from the Class, getting the CodeSource from the ProtectionDomain, and getting the JAR/URL from the CodeSource. The last two parts are easy. It's getting the Class from the StackTrace that is extremely inefficient if not done right. Java 8 took away the best way we had found to do that (which, admittedly, was because we were relying on a non-standard class, which isn't recommended). On 06/17/2013 08:12 AM, Peter Levart wrote: New API could be entirely unrelated to Throwable, if there was support for it in native code. Since there would have to be changes to the native code anyway to support this, why not create a separate API? If I understand this correctly, one part of your code is displaying the resource location of each method's declaring class in the stack trace when such extended stack trace is configured. Therefore you need this API to capture the class-enriched-stack-trace at the time the Throwable instance is constructed. No special Throwable-unrelated API to return the stack trace would be suitable for this purpose then. Yes, this is correct. This is the primary use case that needs satisfying. But for the purpose of displaying the resource location of the class invoking the logging API, you don't need the entire stack trace, just the immediate caller class, right? When @CallerSensitive internal annotation was introduced (and Reflection.getCallerClass(int frames) was dropped), there was some debate about possible introduction of public API that could be used to reveal the caller's class. Perhaps there's still time to create such API for JDK8? At least one part of your logging functionality would be covered with such API. I would love for this to be part of the public API, but since it's annotation-based we wouldn't be able to use it for probably six or seven years. For the foreseeable future Log4j must compile on Java 6. Suppose the API to obtain the immediate caller class (j.l.Class object) was public and fast, so you could do that for each call to the logging API eagerly (after initial check for logging level). Now, could you delay the evaluation of extended stack trace pattern to the latest time possible, when you must format the stack-trace? Would calling Class.forName(name, false, classLoader) for each individual StackTraceElement, using the ClassLoader of the caller class, still present a performance problem? Yes. Our _primary_ concern here is locating the source information for Throwable stack traces. (In fact, if you want, just forget about the source info for the calling class. That's easy. Let's focus JUST on Throwables.) Class.forName() is orders of magnitude slower that SecurityManager#getClassContext() and Reflection#getCallerClass(). And that's not even considering the fact that getClassContext() and getCallerClass() can only fill in _part_ of the stack trace (because they don't line up; one is from Throwable, the other is from current stack). If we could actually obtain the entire stack trace's source info without Class.forName(), it would be even faster. That's why I thought the _simplest_ approach would be to add the getElementClass() method to StackTraceElement ... it works everywhere. If the proposed solution is a StackFrame class instead, that's fine by me. I just need to be able to get the StackFrame[] for a Throwable. Nick
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/16/2013 11:44 PM, Alan Bateman wrote: On 13/06/2013 12:47, Aleksey Shipilev wrote: Friendly reminder for the reviewers. On 06/10/2013 07:53 PM, Aleksey Shipilev wrote: This is the follow-up on the issue Doug identified: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html I had reworked the patch, webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.01/ The current webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.02/ I'm just catching up on this thread, webrev.02 looks very good to me. I can sponsor it. Thanks! I'll appreciate this. One comment on ClassRepositoryHolder is that it's yet another class (we have a proliferation of holder classes since we started using this idiom). It makes me wonder if it might be better to move the constant to ClassRepository. Ok, makes sense. I moved it to ClassRepository itself. Given we have the ClassRepository field in Class, the static initialization will be performed on the first class load. I have no problems having it public, since it is protected enough from the external modifications. The new webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.03/ Testing: - Linux/x86_64/release build OK - Linux/x86_64/release passes java/lang/reflect jtreg - microbenchmark scores are still intact -Aleksey.
Re: RFR (jaxp): 8016133 : Regression: diff. behavior with user-defined SAXParser
Hi Joe, Looks good to me. Not a reviewer - as you know ;-) -- daniel On 6/11/13 5:36 AM, huizhe wang wrote: Hi, This is a quick fix on a regression caused by a previous patch. The XMLReaderFactory uses a class variable (_jarread) to indicate if service file has already been read. Along with this variable, there was another (clsFromJar ) that caches the classname if found in a service file. The 2nd variable and its use were accidentally removed. As a result, the following code would return the 3rd party impl on first call but then fall back to the default impl on subsequent calls because reading service file was skipped when _jarread is true: XMLReader reader = XMLReaderFactory.createXMLReader(); System.out.println(1: + reader.getClass().getName()); XMLReader reader2 = XMLReaderFactory.createXMLReader(); System.out.println(2: + reader2.getClass().getName()); The fix is simply recover the original code. Here's the webrev: http://cr.openjdk.java.net/~joehw/jdk8/8016133/webrev/ Thanks, Joe
Re: There needs to be support for java.time in java.text.MessageFormat
On Jun 15, 2013, at 1:34 PM, Nick Williams wrote: Currently, MessageFormat /only/ supports SimpleDateFormat and instances of java.util.Date or java.util.Calendar for date/time values. Because of this, it's impossible to use Java 8 date/time types with any of the i18n and localization tools. MessageFormat really needs support for types in java.time. Should I file a bug somewhere about this (where?), or is this message enough to get this going? I'm not even sure how it should work. I'm not sure how MessageFormat can know whether a SimpleDateFormat string or a DateTimeFormatter string is being supplied. Possibly, validation of the format string must be delayed until either a Date/Calendar or a java.time type is supplied for formatting. Looks like there _also_ needs to be java.time support in JAXB. I have a POJO with an @XmlElement lastModified of type java.time.Instant, and it just gets printed out lastModified / (the value is ignored). It should be printed out ISO 8601-formatted.
RFR (XS) CR 8014233: java.lang.Thread should have @Contended on TLR fields
Hi, This is the respin of the RFE filed a month ago: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016754.html The webrev is here: http://cr.openjdk.java.net/~shade/8014233/webrev.02/ Testing: - JPRT build passes - Linux x86_64/release passes jdk/java/lang jtreg - vm.quick.testlist, vm.quick-gc.testlist on selected platforms - microbenchmarks, see below The rationale follows. After we merged ThreadLocalRandom state in the thread, we are now missing the padding to prevent false sharing on those heavily-updated fields. While the Thread is already large enough to separate two TLR states for two distinct threads, we can still get the false sharing with other thread fields. There is the benchmark showcasing this: http://cr.openjdk.java.net/~shade/8014233/threadbench.zip There are two test cases: first one is only calling its own TLR with nextInt() and then the current thread's ID, another test calls *another* thread ID, thus inducing the false sharing against another thread's TLR state. On my 2x2 i5 laptop, running Linux x86_64: same:355 +- 1 ops/usec other: 100 +- 5 ops/usec Note the decrease in throughput because of the false sharing. With the patch: same:359 +- 1 ops/usec other: 356 +- 1 ops/usec Note the performance is back. We want to evade these spurious decreases in performance, due to either unlucky memory layout, or the user code (un)intentionally ruining the cache line locality for the updater thread. Thanks, -Aleksey.
Re: There needs to be support for java.time in java.text.MessageFormat
The standard JDK bug tracker should be used to keep track of these. For info, Oracle is in control of integration of JSR-310 into other APIs, not JSR-310. Stephen On 17 June 2013 09:40, Nick Williams nicholas+open...@nicholaswilliams.net wrote: On Jun 15, 2013, at 1:34 PM, Nick Williams wrote: Currently, MessageFormat /only/ supports SimpleDateFormat and instances of java.util.Date or java.util.Calendar for date/time values. Because of this, it's impossible to use Java 8 date/time types with any of the i18n and localization tools. MessageFormat really needs support for types in java.time. Should I file a bug somewhere about this (where?), or is this message enough to get this going? I'm not even sure how it should work. I'm not sure how MessageFormat can know whether a SimpleDateFormat string or a DateTimeFormatter string is being supplied. Possibly, validation of the format string must be delayed until either a Date/Calendar or a java.time type is supplied for formatting. Looks like there _also_ needs to be java.time support in JAXB. I have a POJO with an @XmlElement lastModified of type java.time.Instant, and it just gets printed out lastModified / (the value is ignored). It should be printed out ISO 8601-formatted.
mercurial mq was Re: RFR 8010325 : Remove hash32() method and hash32 int field from java.lang.String
On Jun 14, 2013, at 10:36 PM, Martin Buchholz marti...@google.com wrote: On Thu, Jun 13, 2013 at 12:46 PM, Brent Christian brent.christ...@oracle.com wrote: On 6/12/13 7:55 PM, David Holmes wrote: Something of an aside but ... On 13/06/2013 3:45 AM, Martin Buchholz wrote: Hi Brent, Thanks for doing this. Your webrev does not include mercurial changeset information, which I think is supported by recent webrevs. Given the changeset has to be created after the review is complete most/many people will not have a changeset prepared at review time. That's it exactly. If at all possible, I don't commit until the code has completed code review. I tell webrev to do its thing based on modified files, rather than outgoing changesets. Hmmm I've been using mq for so long it's hard for me to imagine working without it. It allows others to review the mercurial changeset metadata, which is also the best summary to decide whether to review further. +1 mq is the best way i have found to keep multiple patches in flight either in the same queue or using multiple queues, and avoid those annoying merge commits. I wish there were better tooling support in IDEs for it. Paul.
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On Jun 17, 2013, at 10:06 AM, Aleksey Shipilev aleksey.shipi...@oracle.com wrote: On 06/16/2013 11:44 PM, Alan Bateman wrote: On 13/06/2013 12:47, Aleksey Shipilev wrote: Friendly reminder for the reviewers. On 06/10/2013 07:53 PM, Aleksey Shipilev wrote: This is the follow-up on the issue Doug identified: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html I had reworked the patch, webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.01/ The current webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.02/ I'm just catching up on this thread, webrev.02 looks very good to me. I can sponsor it. Thanks! I'll appreciate this. One comment on ClassRepositoryHolder is that it's yet another class (we have a proliferation of holder classes since we started using this idiom). It makes me wonder if it might be better to move the constant to ClassRepository. Ok, makes sense. I moved it to ClassRepository itself. Given we have the ClassRepository field in Class, the static initialization will be performed on the first class load. I have no problems having it public, since it is protected enough from the external modifications. The new webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.03/ Looks good to me. Paul. Testing: - Linux/x86_64/release build OK - Linux/x86_64/release passes java/lang/reflect jtreg - microbenchmark scores are still intact -Aleksey.
Re: There needs to be support for java.time in java.text.MessageFormat
Thanks. I have filed two different bugs for these two different problems. On Jun 17, 2013, at 4:20 AM, Stephen Colebourne wrote: The standard JDK bug tracker should be used to keep track of these. For info, Oracle is in control of integration of JSR-310 into other APIs, not JSR-310. Stephen On 17 June 2013 09:40, Nick Williams nicholas+open...@nicholaswilliams.net wrote: On Jun 15, 2013, at 1:34 PM, Nick Williams wrote: Currently, MessageFormat /only/ supports SimpleDateFormat and instances of java.util.Date or java.util.Calendar for date/time values. Because of this, it's impossible to use Java 8 date/time types with any of the i18n and localization tools. MessageFormat really needs support for types in java.time. Should I file a bug somewhere about this (where?), or is this message enough to get this going? I'm not even sure how it should work. I'm not sure how MessageFormat can know whether a SimpleDateFormat string or a DateTimeFormatter string is being supplied. Possibly, validation of the format string must be delayed until either a Date/Calendar or a java.time type is supplied for formatting. Looks like there _also_ needs to be java.time support in JAXB. I have a POJO with an @XmlElement lastModified of type java.time.Instant, and it just gets printed out lastModified / (the value is ignored). It should be printed out ISO 8601-formatted.
Re: RFR : 8016446 : (m) Add override forEach/replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap
On Jun 14, 2013, at 11:57 PM, Mike Duigou mike.dui...@oracle.com wrote:
I have updated the webrev again. In addition to moving the modification
checks to be consistently after the operation for the most complete
fast-fail behaviour I've also slightly enhanced the Map default to detect
ISE thrown by setValue as a CME condition.
http://cr.openjdk.java.net/~mduigou/JDK-8016446/2/webrev/
There are some places where the indentation looks wonky e.g. HashMap
LinkedHashMap changes.
The LinkedHashMap.forEach/replaceAll methods are checking modCount and throwing
CME before the action is called.
The WeakHashMap.replaceAll method is checking the modCount per bucket, not-per
element.
Are there existing tests for the overriding methods?
Otherwise looks OK.
Paul.
For interference detection the strategy I am advocating is :
- serial non-stream operations
(Collection.forEach/Iterator.forEachRemaining): per-element post-operation
(fast-fail, best-efforts). As Remi has pointed out the failure modes for
collection.forEach() and for(:collection) will differ and it is a conscious
choice to provide superior fast-fail than to match behaviour.
- stream terminal operations, both serial and parallel : at completion if at
all. Having the serial and parallel stream interference detection behaviours
consistent is prioritized over making serial stream behaviour match behaviour
of non-stream iteration methods.
Mike
On Jun 12 2013, at 22:28 , Mike Duigou wrote:
I have updated my webrev with Remi's improvements and some other
improvements to the fast-fail concurrent modification checking.
Revised webrev:
http://cr.openjdk.java.net/~mduigou/JDK-8016446/1/webrev/
Mike
On Jun 12 2013, at 15:49 , Remi Forax wrote:
On 06/12/2013 11:23 PM, Mike Duigou wrote:
Hello all;
This patch adds optimized implementations of Map.forEach and
Map.replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap
http://cr.openjdk.java.net/~mduigou/JDK-8016446/0/webrev/
The implementations traverse the map more efficiently than the default
iterator implementation and sometimes avoid creation of transient
Map.Entry instances. The fast-fail behaviour of the default implementation
is retained.
Mike
Hi Mike,
funnily I was writing HashMap.forEach/LinkedHashMap.forEach at the same
time.
(you need also to override forEach in LinkedHashMap because the one you
inherits from HashMap doesn't use the linked list of entries).
My code is slightly different from yours because I've moved the cases where
the item is a red/black tree out of the fast path
(the tree is created either if you are unlucky, if hashCode is badly
written or if someone forge keys to have collisions)
and does the modCount check for each element because a call to the consumer
can change the underlying map
so you can not do a modCount check only at the end.
Rémi
Here is the diff.
diff -r 6df79b7bae6f src/share/classes/java/util/HashMap.java
--- a/src/share/classes/java/util/HashMap.javaWed Jun 12 09:44:34 2013
+0100
+++ b/src/share/classes/java/util/HashMap.javaThu Jun 13 00:46:05 2013
+0200
@@ -28,6 +28,7 @@
import java.io.*;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
+import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.BiFunction;
import java.util.function.Function;
@@ -2615,6 +2616,54 @@
int capacity() { return table.length; }
float loadFactor() { return loadFactor; }
+
+@Override
+public void forEach(BiConsumer? super K, ? super V action) {
+Objects.requireNonNull(action);
+final int expectedModCount = modCount;
+if (nullKeyEntry != null) {
+forEachNullKey(expectedModCount, action);
+}
+Object[] table = this.table;
+for(int index = 0; index table.length; index++) {
+Object item = table[index];
+if (item == null) {
+continue;
+}
+if (item instanceof HashMap.TreeBin) {
+eachTreeNode(expectedModCount, ((TreeBin)item).first,
action);
+continue;
+}
+@SuppressWarnings(unchecked)
+EntryK,V entry = (EntryK,V)item;
+for (;entry != null; entry = (EntryK,V)entry.next) {
+if (expectedModCount != modCount) {
+throw new ConcurrentModificationException();
+}
+action.accept(entry.key, entry.value);
+}
+}
+}
+
+private void eachTreeNode(int expectedModCount, TreeNodeK,V node,
BiConsumer? super K, ? super V action) {
+while (node != null) {
+if (expectedModCount != modCount) {
+throw new ConcurrentModificationException();
+}
+@SuppressWarnings(unchecked)
+
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/17/2013 10:06 AM, Aleksey Shipilev wrote: On 06/16/2013 11:44 PM, Alan Bateman wrote: On 13/06/2013 12:47, Aleksey Shipilev wrote: Friendly reminder for the reviewers. On 06/10/2013 07:53 PM, Aleksey Shipilev wrote: This is the follow-up on the issue Doug identified: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html I had reworked the patch, webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.01/ The current webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.02/ I'm just catching up on this thread, webrev.02 looks very good to me. I can sponsor it. Thanks! I'll appreciate this. One comment on ClassRepositoryHolder is that it's yet another class (we have a proliferation of holder classes since we started using this idiom). It makes me wonder if it might be better to move the constant to ClassRepository. Ok, makes sense. I moved it to ClassRepository itself. Given we have the ClassRepository field in Class, the static initialization will be performed on the first class load. I have no problems having it public, since it is protected enough from the external modifications. Sorry, Aleksey, just a nit: In getInterfaces(), the method could be written using a single volatile read from ReflectionData.interfaces field instead of two (by introducing local variable). Regards, Peter The new webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.03/ Testing: - Linux/x86_64/release build OK - Linux/x86_64/release passes java/lang/reflect jtreg - microbenchmark scores are still intact -Aleksey.
Re: RFR (jaxp): 8016133 : Regression: diff. behavior with user-defined SAXParser
Looks ok to me too Joe. -Chris. On 17/06/2013 09:17, Daniel Fuchs wrote: Hi Joe, Looks good to me. Not a reviewer - as you know ;-) -- daniel On 6/11/13 5:36 AM, huizhe wang wrote: Hi, This is a quick fix on a regression caused by a previous patch. The XMLReaderFactory uses a class variable (_jarread) to indicate if service file has already been read. Along with this variable, there was another (clsFromJar ) that caches the classname if found in a service file. The 2nd variable and its use were accidentally removed. As a result, the following code would return the 3rd party impl on first call but then fall back to the default impl on subsequent calls because reading service file was skipped when _jarread is true: XMLReader reader = XMLReaderFactory.createXMLReader(); System.out.println(1: + reader.getClass().getName()); XMLReader reader2 = XMLReaderFactory.createXMLReader(); System.out.println(2: + reader2.getClass().getName()); The fix is simply recover the original code. Here's the webrev: http://cr.openjdk.java.net/~joehw/jdk8/8016133/webrev/ Thanks, Joe
Re: RFR (XS) CR 8014233: java.lang.Thread should have @Contended on TLR fields
On Jun 17, 2013, at 11:00 AM, Aleksey Shipilev aleksey.shipi...@oracle.com wrote: Hi, This is the respin of the RFE filed a month ago: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016754.html The webrev is here: http://cr.openjdk.java.net/~shade/8014233/webrev.02/ Looks OK to me (with my lack of reviewing mojo). More out of curiosity: how does this impact the memory-per-Thread with @Contended on fields rather than on the class? Paul.
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 17/06/2013 09:06, Aleksey Shipilev wrote: : Ok, makes sense. I moved it to ClassRepository itself. Given we have the ClassRepository field in Class, the static initialization will be performed on the first class load. I have no problems having it public, since it is protected enough from the external modifications. The new webrev is here: http://cr.openjdk.java.net/~shade/8016236/webrev.03/ This looks to good. I agree with Peter's point about the 2 reads in getInterfaces() although it's unlikely to make a significant difference. If you can create the change-set then I can push this for you today. -Alan
Re: There needs to be support for java.time in java.text.MessageFormat
On 17 June 2013 12:40, Alan Bateman alan.bate...@oracle.com wrote: On 17/06/2013 11:05, Nick Williams wrote: Thanks. I have filed two different bugs for these two different problems. Here they are: 8016743: java.text.MessageFormat does not support java.time.* types 8016742: JAXB does not support java.time.* types Note that JAXB is maintained in an upstream project (they periodically do source drops into OpenJDK). I don't know if support for java.time requires API changes or not but just to mention that JAXB is tied to a standalone JSR and I think they the requirement to be able to drop-in into jdk7 builds. That could be a problem, but its really a process one. It shouldn't be users that suffer as a result of issues like that (but there isn't anything I can do about it other than wave my hands...) Stephen
Re: Cleanup of some doclint warnings in java.lang
On 17/06/2013 04:48, Joe Darcy wrote:
Hello,
Please review the patch below which resolves a subset of the doclint
warnings in java.lang.
Thanks,
-Joe
One comment on Double is that you've removed the code to fix the
nesting issue. Did you consider replacing this with pre{@code ...
}/pre so that you've got {@code ..} around the code fragment. That
would avoid needing to check it for escaping.
Otherwise looks fine to me.
-Alan.
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/17/2013 02:20 PM, Peter Levart wrote: Sorry, Aleksey, just a nit: In getInterfaces(), the method could be written using a single volatile read from ReflectionData.interfaces field instead of two (by introducing local variable). Yes, thanks! Nothing to be sorry of, I'll cover this. -Aleksey.
Re: Cleanup of some doclint warnings in java.lang
Looks fine to me Joe,
Quite trivially, and optionally, the '@since 1.8 ' could be moved to
after the @return in Short.java L406
-Chris.
On 17/06/2013 04:48, Joe Darcy wrote:
Hello,
Please review the patch below which resolves a subset of the doclint
warnings in java.lang.
Thanks,
-Joe
diff -r 45a3584bfacf src/share/classes/java/lang/Boolean.java
--- a/src/share/classes/java/lang/Boolean.java Fri Jun 14 15:14:56 2013
+0400
+++ b/src/share/classes/java/lang/Boolean.java Sun Jun 16 20:46:52 2013
-0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1994, 2011, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1994, 2013, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -205,9 +205,9 @@
* Returns a hash code for a {@code boolean} value; compatible with
* {@code Boolean.hashCode()}.
*
+ * @param value the value to hash
+ * @return a hash code value for a {@code boolean} value.
* @since 1.8
- *
- * @return a hash code value for a {@code boolean} value.
*/
public static int hashCode(boolean value) {
return value ? 1231 : 1237;
diff -r 45a3584bfacf src/share/classes/java/lang/Byte.java
--- a/src/share/classes/java/lang/Byte.java Fri Jun 14 15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/Byte.java Sun Jun 16 20:46:52 2013 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -398,9 +398,9 @@
* Returns a hash code for a {@code byte} value; compatible with
* {@code Byte.hashCode()}.
*
+ * @param value the value to hash
+ * @return a hash code value for a {@code byte} value.
* @since 1.8
- *
- * @return a hash code value for a {@code byte} value.
*/
public static int hashCode(byte value) {
return (int)value;
diff -r 45a3584bfacf src/share/classes/java/lang/Class.java
--- a/src/share/classes/java/lang/Class.java Fri Jun 14 15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/Class.java Sun Jun 16 20:46:52 2013 -0700
@@ -3141,6 +3141,8 @@
* could not be checked at runtime (because generic types are implemented
* by erasure).
*
+ * @param U the type to cast this class object to
+ * @param clazz the class of the type to cast this class object to
* @return this {@code Class} object, cast to represent a subclass of
* the specified class object.
* @throws ClassCastException if this {@code Class} object does not
@@ -3296,6 +3298,7 @@
* If this Class represents either the Object class, an interface type, an
* array type, a primitive type, or void, the return value is null.
*
+ * @return an object representing the superclass
* @since 1.8
*/
public AnnotatedType getAnnotatedSuperclass() {
@@ -3327,6 +3330,7 @@
* If this Class represents either the Object class, an array type, a
* primitive type, or void, the return value is an array of length 0.
*
+ * @return an array representing the superinterfaces
* @since 1.8
*/
public AnnotatedType[] getAnnotatedInterfaces() {
diff -r 45a3584bfacf src/share/classes/java/lang/Double.java
--- a/src/share/classes/java/lang/Double.java Fri Jun 14 15:14:56 2013
+0400
+++ b/src/share/classes/java/lang/Double.java Sun Jun 16 20:46:52 2013
-0700
@@ -453,7 +453,6 @@
* a {@code NumberFormatException} be thrown, the regular
* expression below can be used to screen the input string:
*
- * code
* pre
* final String Digits = (\\p{Digit}+);
* final String HexDigits = (\\p{XDigit}+);
@@ -500,7 +499,6 @@
* // Perform suitable alternative action
* }
* /pre
- * /code
*
* @param s the string to be parsed.
* @return a {@code Double} object holding the value
@@ -756,9 +754,9 @@
* Returns a hash code for a {@code double} value; compatible with
* {@code Double.hashCode()}.
*
+ * @param value the value to hash
+ * @return a hash code value for a {@code double} value.
* @since 1.8
- *
- * @return a hash code value for a {@code double} value.
*/
public static int hashCode(double value) {
long bits = doubleToLongBits(value);
diff -r 45a3584bfacf src/share/classes/java/lang/Float.java
--- a/src/share/classes/java/lang/Float.java Fri Jun 14 15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/Float.java Sun Jun 16 20:46:52 2013 -0700
@@ -664,9 +664,9 @@
* Returns a hash code for a {@code float} value; compatible with
* {@code Float.hashCode()}.
*
+ * @param value the value to hash
+ * @return a hash code value for a {@code float} value.
* @since 1.8
- *
- * @return a hash code value for a {@code float} value.
*/
public static int hashCode(float value) {
return floatToIntBits(value);
diff -r 45a3584bfacf src/share/classes/java/lang/Integer.java
--- a/src/share/classes/java/lang/Integer.java Fri Jun 14 15:14:56 2013
+0400
+++ b/src/share/classes/java/lang/Integer.java Sun Jun 16 20:46:52
Re: Cleanup of doclint warnings in java.lang.{annotation, reflect}
Looks ok to me.
-Chris.
On 17/06/2013 00:34, Joe Darcy wrote:
Hello,
Please review the diff below of changes to get java.lang.annotation and
java.lang.reflect clean on doclint warnings.
I soon plan to send out another patch to cleanup java.lang; I'll file
one or more bugs to cover this work depending on how reviews come in.
Thanks,
-Joe
diff -r 45a3584bfacf src/share/classes/java/lang/annotation/Annotation.java
--- a/src/share/classes/java/lang/annotation/Annotation.java Fri Jun 14
15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/annotation/Annotation.java Sun Jun 16
16:34:09 2013 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -126,6 +126,7 @@
/**
* Returns the annotation type of this annotation.
+ * @return the annotation type of this annotation
*/
Class? extends Annotation annotationType();
}
diff -r 45a3584bfacf src/share/classes/java/lang/annotation/Repeatable.java
--- a/src/share/classes/java/lang/annotation/Repeatable.java Fri Jun 14
15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/annotation/Repeatable.java Sun Jun 16
16:34:09 2013 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -43,6 +43,7 @@
/**
* Indicates the emcontaining annotation type/em for the
* repeatable annotation type.
+ * @return the containing annotation type
*/
Class? extends Annotation value();
}
diff -r 45a3584bfacf src/share/classes/java/lang/annotation/Retention.java
--- a/src/share/classes/java/lang/annotation/Retention.java Fri Jun 14
15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/annotation/Retention.java Sun Jun 16
16:34:09 2013 -0700
@@ -44,5 +44,9 @@
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Retention {
+ /**
+ * Returns the retention policy.
+ * @return the retention policy
+ */
RetentionPolicy value();
}
diff -r 45a3584bfacf src/share/classes/java/lang/annotation/Target.java
--- a/src/share/classes/java/lang/annotation/Target.java Fri Jun 14
15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/annotation/Target.java Sun Jun 16
16:34:09 2013 -0700
@@ -67,5 +67,11 @@
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Target {
+ /**
+ * Returns an array of the kinds of elements an annotation type
+ * can be applied to.
+ * @return an array of the kinds of elements an annotation type
+ * can be applied to
+ */
ElementType[] value();
}
diff -r 45a3584bfacf
src/share/classes/java/lang/reflect/AnnotatedElement.java
--- a/src/share/classes/java/lang/reflect/AnnotatedElement.java Fri Jun
14 15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/reflect/AnnotatedElement.java Sun Jun
16 16:34:09 2013 -0700
@@ -130,6 +130,7 @@
* Returns this element's annotation for the specified type if
* such an annotation is present, else null.
*
+ * @param T the type of the annotation to query for and return if
present
* @param annotationClass the Class object corresponding to the
* annotation type
* @return this element's annotation for the specified annotation type if
@@ -154,6 +155,7 @@
* The caller of this method is free to modify the returned array; it will
* have no effect on the arrays returned to other callers.
*
+ * @param T the type of the annotation to query for and return if
present
* @param annotationClass the Class object corresponding to the
* annotation type
* @return all this element's annotations for the specified annotation
type if
@@ -184,6 +186,7 @@
* This method ignores inherited annotations. (Returns null if no
* annotations are directly present on this element.)
*
+ * @param T the type of the annotation to query for and return if
present
* @param annotationClass the Class object corresponding to the
* annotation type
* @return this element's annotation for the specified annotation type if
@@ -209,6 +212,8 @@
* The caller of this method is free to modify the returned array; it will
* have no effect on the arrays returned to other callers.
*
+ * @param T the type of the annotation to query for and return
+ * if directly present
* @param annotationClass the Class object corresponding to the
* annotation type
* @return all this element's annotations for the specified annotation
type if
diff -r 45a3584bfacf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.java Fri Jun 14
15:14:56 2013 +0400
+++ b/src/share/classes/java/lang/reflect/Executable.java Sun Jun 16
16:34:09 2013 -0700
@@ -384,6 +384,8 @@
/**
* Returns a
Re: RFR (XS) CR 8014233: java.lang.Thread should have @Contended on TLR fields
Looks fine to me Aleksey. Let me know if you need a sponsor. -Chris. On 17/06/2013 10:00, Aleksey Shipilev wrote: Hi, This is the respin of the RFE filed a month ago: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016754.html The webrev is here: http://cr.openjdk.java.net/~shade/8014233/webrev.02/ Testing: - JPRT build passes - Linux x86_64/release passes jdk/java/lang jtreg - vm.quick.testlist, vm.quick-gc.testlist on selected platforms - microbenchmarks, see below The rationale follows. After we merged ThreadLocalRandom state in the thread, we are now missing the padding to prevent false sharing on those heavily-updated fields. While the Thread is already large enough to separate two TLR states for two distinct threads, we can still get the false sharing with other thread fields. There is the benchmark showcasing this: http://cr.openjdk.java.net/~shade/8014233/threadbench.zip There are two test cases: first one is only calling its own TLR with nextInt() and then the current thread's ID, another test calls *another* thread ID, thus inducing the false sharing against another thread's TLR state. On my 2x2 i5 laptop, running Linux x86_64: same:355 +- 1 ops/usec other: 100 +- 5 ops/usec Note the decrease in throughput because of the false sharing. With the patch: same:359 +- 1 ops/usec other: 356 +- 1 ops/usec Note the performance is back. We want to evade these spurious decreases in performance, due to either unlucky memory layout, or the user code (un)intentionally ruining the cache line locality for the updater thread. Thanks, -Aleksey.
hg: jdk8/tl/jdk: 8016747: Replace deprecated PlatformLogger isLoggable(int) with isLoggable(Level)
Changeset: adf70cb48ce0 Author:chegar Date: 2013-06-17 14:09 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/adf70cb48ce0 8016747: Replace deprecated PlatformLogger isLoggable(int) with isLoggable(Level) Reviewed-by: darcy ! src/macosx/classes/sun/lwawt/LWComponentPeer.java ! src/macosx/classes/sun/lwawt/LWWindowPeer.java ! src/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java ! src/share/classes/java/awt/AWTEvent.java ! src/share/classes/java/awt/AttributeValue.java ! src/share/classes/java/awt/Component.java ! src/share/classes/java/awt/Container.java ! src/share/classes/java/awt/ContainerOrderFocusTraversalPolicy.java ! src/share/classes/java/awt/Cursor.java ! src/share/classes/java/awt/DefaultKeyboardFocusManager.java ! src/share/classes/java/awt/EventDispatchThread.java ! src/share/classes/java/awt/EventQueue.java ! src/share/classes/java/awt/KeyboardFocusManager.java ! src/share/classes/java/awt/SplashScreen.java ! src/share/classes/java/awt/Toolkit.java ! src/share/classes/java/awt/WaitDispatchSupport.java ! src/share/classes/java/awt/Window.java ! src/share/classes/java/awt/event/InputEvent.java ! src/share/classes/java/net/CookieManager.java ! src/share/classes/java/util/Currency.java ! src/share/classes/javax/swing/BufferStrategyPaintManager.java ! src/share/classes/javax/swing/SortingFocusTraversalPolicy.java ! src/share/classes/sun/awt/AWTAutoShutdown.java ! src/share/classes/sun/awt/DebugSettings.java ! src/share/classes/sun/awt/KeyboardFocusManagerPeerImpl.java ! src/share/classes/sun/awt/ScrollPaneWheelScroller.java ! src/share/classes/sun/awt/SunDisplayChanger.java ! src/share/classes/sun/awt/SunGraphicsCallback.java ! src/share/classes/sun/awt/SunToolkit.java ! src/share/classes/sun/awt/datatransfer/DataTransferer.java ! src/share/classes/sun/awt/dnd/SunDropTargetContextPeer.java ! src/share/classes/sun/awt/im/InputContext.java ! src/share/classes/sun/font/SunFontManager.java ! src/share/classes/sun/net/ftp/impl/FtpClient.java ! src/share/classes/sun/net/www/http/HttpClient.java ! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java ! src/share/classes/sun/net/www/protocol/http/NTLMAuthenticationProxy.java ! src/share/classes/sun/net/www/protocol/http/Negotiator.java ! src/share/classes/sun/net/www/protocol/https/HttpsClient.java ! src/solaris/classes/sun/awt/X11/ListHelper.java ! src/solaris/classes/sun/awt/X11/UnsafeXDisposerRecord.java ! src/solaris/classes/sun/awt/X11/XAWTXSettings.java ! src/solaris/classes/sun/awt/X11/XBaseMenuWindow.java ! src/solaris/classes/sun/awt/X11/XBaseWindow.java ! src/solaris/classes/sun/awt/X11/XCheckboxPeer.java ! src/solaris/classes/sun/awt/X11/XChoicePeer.java ! src/solaris/classes/sun/awt/X11/XComponentPeer.java ! src/solaris/classes/sun/awt/X11/XContentWindow.java ! src/solaris/classes/sun/awt/X11/XDecoratedPeer.java ! src/solaris/classes/sun/awt/X11/XDnDDragSourceProtocol.java ! src/solaris/classes/sun/awt/X11/XDnDDropTargetProtocol.java ! src/solaris/classes/sun/awt/X11/XDragSourceContextPeer.java ! src/solaris/classes/sun/awt/X11/XDropTargetContextPeer.java ! src/solaris/classes/sun/awt/X11/XDropTargetProtocol.java ! src/solaris/classes/sun/awt/X11/XDropTargetRegistry.java ! src/solaris/classes/sun/awt/X11/XEmbedCanvasPeer.java ! src/solaris/classes/sun/awt/X11/XEmbedClientHelper.java ! src/solaris/classes/sun/awt/X11/XEmbedHelper.java ! src/solaris/classes/sun/awt/X11/XEmbedServerTester.java ! src/solaris/classes/sun/awt/X11/XEmbeddedFramePeer.java ! src/solaris/classes/sun/awt/X11/XErrorHandlerUtil.java ! src/solaris/classes/sun/awt/X11/XFileDialogPeer.java ! src/solaris/classes/sun/awt/X11/XFramePeer.java ! src/solaris/classes/sun/awt/X11/XIconWindow.java ! src/solaris/classes/sun/awt/X11/XInputMethod.java ! src/solaris/classes/sun/awt/X11/XKeyboardFocusManagerPeer.java ! src/solaris/classes/sun/awt/X11/XListPeer.java ! src/solaris/classes/sun/awt/X11/XMSelection.java ! src/solaris/classes/sun/awt/X11/XMenuBarPeer.java ! src/solaris/classes/sun/awt/X11/XMenuPeer.java ! src/solaris/classes/sun/awt/X11/XMenuWindow.java ! src/solaris/classes/sun/awt/X11/XNETProtocol.java ! src/solaris/classes/sun/awt/X11/XPopupMenuPeer.java ! src/solaris/classes/sun/awt/X11/XProtocol.java ! src/solaris/classes/sun/awt/X11/XScrollbar.java ! src/solaris/classes/sun/awt/X11/XScrollbarPeer.java ! src/solaris/classes/sun/awt/X11/XSystemTrayPeer.java ! src/solaris/classes/sun/awt/X11/XTextFieldPeer.java ! src/solaris/classes/sun/awt/X11/XToolkit.java ! src/solaris/classes/sun/awt/X11/XTrayIconPeer.java ! src/solaris/classes/sun/awt/X11/XWINProtocol.java ! src/solaris/classes/sun/awt/X11/XWM.java ! src/solaris/classes/sun/awt/X11/XWindow.java ! src/solaris/classes/sun/awt/X11/XWindowPeer.java ! src/solaris/classes/sun/awt/X11GraphicsEnvironment.java ! src/solaris/classes/sun/awt/X11InputMethod.java ! src/windows/classes/sun/awt/windows/WComponentPeer.java ! src/windows/classes/sun/awt/windows/WDesktopProperties.java !
Re: There needs to be support for java.time in java.text.MessageFormat
On Jun 17, 2013, at 6:53 AM, Stephen Colebourne wrote: On 17 June 2013 12:40, Alan Bateman alan.bate...@oracle.com wrote: On 17/06/2013 11:05, Nick Williams wrote: Thanks. I have filed two different bugs for these two different problems. Here they are: 8016743: java.text.MessageFormat does not support java.time.* types 8016742: JAXB does not support java.time.* types Note that JAXB is maintained in an upstream project (they periodically do source drops into OpenJDK). I don't know if support for java.time requires API changes or not but just to mention that JAXB is tied to a standalone JSR and I think they the requirement to be able to drop-in into jdk7 builds. That could be a problem, but its really a process one. It shouldn't be users that suffer as a result of issues like that (but there isn't anything I can do about it other than wave my hands...) Stephen ^^ What Stephen said. :-)
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 17/06/2013 13:32, Aleksey Shipilev wrote: On 06/17/2013 03:44 PM, Alan Bateman wrote: This looks to good. I agree with Peter's point about the 2 reads in getInterfaces() although it's unlikely to make a significant difference. Oh, it can have the difference on non-x86 hardware and/or in optimized code. This nit is fixed here: http://cr.openjdk.java.net/~shade/8016236/webrev.04/ I did the same testing as before: Linux x86_64/release builds and runs jdk/test/java/lang/reflect jtreg successfully; the performance is still there. If you can create the change-set then I can push this for you today. Here: http://cr.openjdk.java.net/~shade/8016236/8016236.changeset Although only Alan counts as official reviewer; do we need another one? The updated webrev looks good for me. For the jdk repository then only one reviewer with reviewer role is required. I see you have Peter and Paul listed in a Reviewed-by line so I think we are good. Do you want Doug listed too as this started out as a patch from Doug. -Alan
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/17/2013 05:22 PM, Alan Bateman wrote: For the jdk repository then only one reviewer with reviewer role is required. I see you have Peter and Paul listed in a Reviewed-by line so I think we are good. Do you want Doug listed too as this started out as a patch from Doug. Ouch! My bad. Doug had not reviewed the final patch, and the final patch is significantly different from the original one. Do we still want to put Doug to Contributed-by, or is there some other tag useful in cases like these? -Aleksey.
RFR: 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently
Hi, Please find below a webrev for fixing 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently http://cr.openjdk.java.net/~dfuchs/JDK-8014218/webrev.00/ Without the fix - it's quite easy to make the test fail - especially if you copy paste the @run line a few times. This test is supposed to detect a deadlock in Logger initialization, but it detects too many false positive. An analysis of stack traces that the test dumps when it fails shows that there is actually no deadlock, since one of the two supposed deadlock threads is RUNNABLE. However - this - I think - indicates a bug in ThreadMXBean.findDeadlockedThreads(). I have rewritten the test to sanitize the results before and after calling ThreadMXBean.findDeadlockedThreads(). Namely - the test will call ThreadMXBean.findDeadlockedThreads() only if the two threads it monitors are simultaneously blocked (obtained by examining ThreadMXBean.getThreadInfo(long[])), and will verify that all threads found in deadlock by ThreadMXBean.findDeadlockedThreads() are indeed blocked. Now - I'm not sure whether this is a good thing or not: pros: it makes it possible to run this test again - and have a greater confidence that if it fails it's because of a genuine deadlock, which is desirable. cons: it 'hides' a possible bug in ThreadMXBean.findDeadlockedThreads() as it appears the test was testing ThreadMXBean.findDeadlockedThreads() more than (or as much as) it was testing the Logger implementation. Does it make sense to push this fix and log a bug against ThreadMXBean? Or should I instead plan to add this test to the problem list (and re-qualify the bug as an issue in ThreadMXBean)? guidance appreciated! best regards, -- daniel
Re: RFR (XS) CR 8014233: java.lang.Thread should have @Contended on TLR fields
Thanks! I do need a sponsor. -Aleksey. On 06/17/2013 04:52 PM, Chris Hegarty wrote: Looks fine to me Aleksey. Let me know if you need a sponsor. -Chris. On 17/06/2013 10:00, Aleksey Shipilev wrote: Hi, This is the respin of the RFE filed a month ago: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016754.html The webrev is here: http://cr.openjdk.java.net/~shade/8014233/webrev.02/ Testing: - JPRT build passes - Linux x86_64/release passes jdk/java/lang jtreg - vm.quick.testlist, vm.quick-gc.testlist on selected platforms - microbenchmarks, see below The rationale follows. After we merged ThreadLocalRandom state in the thread, we are now missing the padding to prevent false sharing on those heavily-updated fields. While the Thread is already large enough to separate two TLR states for two distinct threads, we can still get the false sharing with other thread fields. There is the benchmark showcasing this: http://cr.openjdk.java.net/~shade/8014233/threadbench.zip There are two test cases: first one is only calling its own TLR with nextInt() and then the current thread's ID, another test calls *another* thread ID, thus inducing the false sharing against another thread's TLR state. On my 2x2 i5 laptop, running Linux x86_64: same:355 +- 1 ops/usec other: 100 +- 5 ops/usec Note the decrease in throughput because of the false sharing. With the patch: same:359 +- 1 ops/usec other: 356 +- 1 ops/usec Note the performance is back. We want to evade these spurious decreases in performance, due to either unlucky memory layout, or the user code (un)intentionally ruining the cache line locality for the updater thread. Thanks, -Aleksey.
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/17/13 09:25, Aleksey Shipilev wrote: Ouch! My bad. Doug had not reviewed the final patch, and the final patch is significantly different from the original one. Do we still want to put Doug to Contributed-by, or is there some other tag useful in cases like these? It looks fine to me. Thanks for the thorough care and testing. I don't think we distinguish idea-cotributed-by vs code-contributed-by :-) -Doug
Re: RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
On 06/17/2013 05:33 PM, Doug Lea wrote: On 06/17/13 09:25, Aleksey Shipilev wrote: Ouch! My bad. Doug had not reviewed the final patch, and the final patch is significantly different from the original one. Do we still want to put Doug to Contributed-by, or is there some other tag useful in cases like these? It looks fine to me. Thanks for the thorough care and testing. I don't think we distinguish idea-cotributed-by vs code-contributed-by :-) Thanks! I had updated the changeset capturing both: http://cr.openjdk.java.net/~shade/8016236/8016236.changeset -Aleksey.
Re: RFR: 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently
On 17/06/2013 14:21, Daniel Fuchs wrote: Hi, Please find below a webrev for fixing 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently http://cr.openjdk.java.net/~dfuchs/JDK-8014218/webrev.00/ Without the fix - it's quite easy to make the test fail - especially if you copy paste the @run line a few times. This test is supposed to detect a deadlock in Logger initialization, but it detects too many false positive. An analysis of stack traces that the test dumps when it fails shows that there is actually no deadlock, since one of the two supposed deadlock threads is RUNNABLE. However - this - I think - indicates a bug in ThreadMXBean.findDeadlockedThreads(). I have rewritten the test to sanitize the results before and after calling ThreadMXBean.findDeadlockedThreads(). Namely - the test will call ThreadMXBean.findDeadlockedThreads() only if the two threads it monitors are simultaneously blocked (obtained by examining ThreadMXBean.getThreadInfo(long[])), and will verify that all threads found in deadlock by ThreadMXBean.findDeadlockedThreads() are indeed blocked. Now - I'm not sure whether this is a good thing or not: pros: it makes it possible to run this test again - and have a greater confidence that if it fails it's because of a genuine deadlock, which is desirable. cons: it 'hides' a possible bug in ThreadMXBean.findDeadlockedThreads() as it appears the test was testing ThreadMXBean.findDeadlockedThreads() more than (or as much as) it was testing the Logger implementation. Does it make sense to push this fix and log a bug against ThreadMXBean? Or should I instead plan to add this test to the problem list (and re-qualify the bug as an issue in ThreadMXBean)? I'd be ok to simply file a bug against ThreadMXBean, and put the test on the ProblemList.txt for now. It should then be revisited with the bug in ThreadMXBean. -Chris. guidance appreciated! best regards, -- daniel
Re: RFR 8012647: Add Arrays.parallelPrefix (prefix sum, scan, cumulative sum)
I haven't seen any convincing arguments to move away from parallelPrefix, or any strong objections to it, so I'll go ahead an finalize the API. -Chris. On 13/06/2013 10:30, Chris Hegarty wrote: On 06/12/2013 05:07 PM, Alan Bateman wrote: On 12/06/2013 15:50, Chris Hegarty wrote: ... Is the name final? Just curious if other names such as parallelScan have been considered (and discarded). I think we should be open to discussing the name, but I will have to defer to Doug as to whether other names were considered/discarded. It seems the convention we are adopting for naming these new methods, whose implementation is parallelized, is to prefix the name with 'parallel' ( parallelSort, parallelSetAll ). Sounds fine. Otherwise I would have preferred something like prefixScan(..). Options so far: parallelPrefix(..) // what we have today parallelScan(..) parallelPrefixScan(..) // too long winded?? Cumulates in parallel each element ..., I guess I would put a comma before and after in parallel. Yes, I will add this. Otherwise the API looks good to me. Thanks Alan, -Chris. -Alan.
hg: jdk8/tl/jdk: 8016236: Class.getGenericInterfaces performance improvement
Changeset: b0cfde1e70e9 Author:shade Date: 2013-06-17 16:28 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b0cfde1e70e9 8016236: Class.getGenericInterfaces performance improvement Summary: cache more reflective data and lookup results. Reviewed-by: alanb, plevart, psandoz, dl Contributed-by: Doug Lea d...@cs.oswego.edu, Aleksey Shipilev aleksey.shipi...@oracle.com ! src/share/classes/java/lang/Class.java ! src/share/classes/sun/reflect/generics/repository/ClassRepository.java ! src/share/native/java/lang/Class.c
Re: RFR: 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently
On 6/17/13 4:22 PM, Chris Hegarty wrote: On 17/06/2013 14:21, Daniel Fuchs wrote: Hi, Please find below a webrev for fixing 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently http://cr.openjdk.java.net/~dfuchs/JDK-8014218/webrev.00/ Without the fix - it's quite easy to make the test fail - especially if you copy paste the @run line a few times. This test is supposed to detect a deadlock in Logger initialization, but it detects too many false positive. An analysis of stack traces that the test dumps when it fails shows that there is actually no deadlock, since one of the two supposed deadlock threads is RUNNABLE. However - this - I think - indicates a bug in ThreadMXBean.findDeadlockedThreads(). I have rewritten the test to sanitize the results before and after calling ThreadMXBean.findDeadlockedThreads(). Namely - the test will call ThreadMXBean.findDeadlockedThreads() only if the two threads it monitors are simultaneously blocked (obtained by examining ThreadMXBean.getThreadInfo(long[])), and will verify that all threads found in deadlock by ThreadMXBean.findDeadlockedThreads() are indeed blocked. Now - I'm not sure whether this is a good thing or not: pros: it makes it possible to run this test again - and have a greater confidence that if it fails it's because of a genuine deadlock, which is desirable. cons: it 'hides' a possible bug in ThreadMXBean.findDeadlockedThreads() as it appears the test was testing ThreadMXBean.findDeadlockedThreads() more than (or as much as) it was testing the Logger implementation. Does it make sense to push this fix and log a bug against ThreadMXBean? Or should I instead plan to add this test to the problem list (and re-qualify the bug as an issue in ThreadMXBean)? I'd be ok to simply file a bug against ThreadMXBean, and put the test on the ProblemList.txt for now. It should then be revisited with the bug in ThreadMXBean. -Chris. Thanks Chris. As it turns out there's already a bug for ThreadMXBean. So I've linked the two together. best regards, -- daniel guidance appreciated! best regards, -- daniel
Re: mercurial mq was Re: RFR 8010325 : Remove hash32() method and hash32 int field from java.lang.String
2013/6/17 2:32 -0700, brian.burkhal...@oracle.com: On Jun 17, 2013, at 2:18 AM, Paul Sandoz wrote: +1 mq is the best way i have found to keep multiple patches in flight either in the same queue or using multiple queues, and avoid those annoying merge commits. +1 I've started using the -l option of qnew to create the changeset comment. This can later be updated by directly editing the patch file in .hg/patches, for example to specify the final reviewer(s). To update the changeset comment you can also use the -e, -m, or -l option with the qrefresh subcommand. - Mark
hg: jdk8/tl: 8016572: Pass CONCURRENCY=$(JOBS) to test/Makefile
Changeset: f8770fe60d53 Author:mduigou Date: 2013-06-17 09:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/f8770fe60d53 8016572: Pass CONCURRENCY=$(JOBS) to test/Makefile Reviewed-by: alanb, erikj ! common/makefiles/Main.gmk
Re: mercurial mq was Re: RFR 8010325 : Remove hash32() method and hash32 int field from java.lang.String
On Jun 17, 2013, at 9:43 AM, mark.reinh...@oracle.com wrote: I've started using the -l option of qnew to create the changeset comment. This can later be updated by directly editing the patch file in .hg/patches, for example to specify the final reviewer(s). To update the changeset comment you can also use the -e, -m, or -l option with the qrefresh subcommand. Ah, that's better. Thanks, Brian
Re: Inefficient code of String.indexOf(String)
For sufficiently large strings, indexOf can also be profitably parallelized. David On 2013-06-17, at 2:14 AM, Martin Buchholz marti...@google.com wrote: You are not the first person to have this idea. It is unlikely that you will succeed in changing the algorithm, because the jit-optimized brute-force algorithm is almost always faster. But go ahead and prove me wrong! On Sun, Jun 16, 2013 at 3:53 AM, Anubhav Chaturvedi mailforanub...@gmail.com wrote: Hello, I have recently started to explore the source code and am new to the open source community. I observed that in String.class within java.lang , the indexOf method, line 1715, uses the bruteforce approach when it comes to string matching. This method is used by the contains(CharSequence) method. There are a number of algorithms that can perform the task more efficiently. I would like to bring the required changes and needed your advice on this.
Re: RFR: 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently
On 6/17/13 7:11 PM, Mandy Chung wrote: FYI. Staffan has a fix in hotspot for JDK-8016304 the ThreadMXBean issue [1]. Thanks Mandy. Yes I saw that. I will wait for Staffan's fix - revert my changes, and run the test again - and see what I get. Alternatively I could now modify the test to try detect JDK-8016304 too and throw an exception pointing at 8016304 if the thread ids returned by findDeadlockedThread do not seem to indicate a deadlock. -- daniel Thanks Mandy [1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-June/010194.html On 6/17/2013 6:21 AM, Daniel Fuchs wrote: Hi, Please find below a webrev for fixing 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently http://cr.openjdk.java.net/~dfuchs/JDK-8014218/webrev.00/ Without the fix - it's quite easy to make the test fail - especially if you copy paste the @run line a few times. This test is supposed to detect a deadlock in Logger initialization, but it detects too many false positive. An analysis of stack traces that the test dumps when it fails shows that there is actually no deadlock, since one of the two supposed deadlock threads is RUNNABLE. However - this - I think - indicates a bug in ThreadMXBean.findDeadlockedThreads(). I have rewritten the test to sanitize the results before and after calling ThreadMXBean.findDeadlockedThreads(). Namely - the test will call ThreadMXBean.findDeadlockedThreads() only if the two threads it monitors are simultaneously blocked (obtained by examining ThreadMXBean.getThreadInfo(long[])), and will verify that all threads found in deadlock by ThreadMXBean.findDeadlockedThreads() are indeed blocked. Now - I'm not sure whether this is a good thing or not: pros: it makes it possible to run this test again - and have a greater confidence that if it fails it's because of a genuine deadlock, which is desirable. cons: it 'hides' a possible bug in ThreadMXBean.findDeadlockedThreads() as it appears the test was testing ThreadMXBean.findDeadlockedThreads() more than (or as much as) it was testing the Logger implementation. Does it make sense to push this fix and log a bug against ThreadMXBean? Or should I instead plan to add this test to the problem list (and re-qualify the bug as an issue in ThreadMXBean)? guidance appreciated! best regards, -- daniel
Re: Code Review Request: 8016698: Cleanup overrides warning in sun/tools/ClassDeclaration.java
On 6/14/2013 11:56 PM, Alan Bateman wrote: On 14/06/2013 23:59, Kurchi Hazra wrote: Hi, This is to elimnate the overrides warning from ClassDeclaration.java. This class is used by rmi, but sun/tools/java and sun/tools/javac also heavily uses it, let me know if anyone else should also be reviewing it. Bug: http://bugs.sun.com/view_bug.do?bug_id=8016698 [To appear] Webrev: http://cr.openjdk.java.net/~khazra/8016698/webrev.00/ Thanks, Kurchi This looks okay for the purposes of squashing the warning but the existing equals method does look suspect. It might not matter for the rmic usage but as the equals doesn't take account of the the definition then it looks to me that it consider very different ClassDeclarations as equals. It would be good to understand how it is used to see whether we have an issue here or not. Thanks for the review. If you analyse definition, it is not constant and changes as the state of the class changes (whether binary class is loaded, whether it has been compiled and so on) - and so does the status field. The type field looks like the only good candidate to use in the equals method - so the equals() looks correct to me. Now the question is whether there is any need of overriding Object.equals() at all, that is, whether each ClassDeclaration object should be treated as a unique one - I am looking around in the source code to check that. Thanks, - Kurchi
RFR 7131192: BigInteger.doubleValue() is depressingly slow
I am unable at the moment to copy anything to cr.openjdk.java.net so I am
including the webrev as a patch below. I'll copy it to the online location as
soon as possible.
The patch included at the end of this message fixes this issue
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7131192
Code review of the source and accompanying test was effected and all pertinent
regression tests passed. Previous performance testing showed a massive
improvement:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-February/014697.html
Note that as correctness testing of 7131192 depends on the patch
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017958.html
having already been applied, the present fix cannot be integrated until this
prior patch has been applied.
Thanks,
Brian
# HG changeset patch
# Parent 35d5a218004fb1fc64c92fd3151d0a48d4f378e9
7131192: BigInteger.doubleValue() is depressingly slow.
Summary: Replace invocations Double.parseDouble(toString()) and
Float.parseFloat(toString()) with direct implementation of conversion from
signum and mag.
Reviewed-by: TBD
Contributed-by: Louis Wasserman lowas...@google.com
diff -r 35d5a218004f src/share/classes/java/math/BigInteger.java
--- a/src/share/classes/java/math/BigInteger.java Thu Jun 13 14:18:52
2013 -0700
+++ b/src/share/classes/java/math/BigInteger.java Fri Jun 14 14:10:43
2013 -0700
@@ -33,6 +33,9 @@
import java.io.*;
import java.util.Arrays;
+import sun.misc.DoubleConsts;
+import sun.misc.FloatConsts;
+
/**
* Immutable arbitrary-precision integers. All operations behave as if
* BigIntegers were represented in two's-complement notation (like Java's
@@ -2966,8 +2969,72 @@
* @return this BigInteger converted to a {@code float}.
*/
public float floatValue() {
-// Somewhat inefficient, but guaranteed to work.
-return Float.parseFloat(this.toString());
+if (signum == 0) {
+return 0.0f;
+}
+
+int exponent = ((mag.length - 1) 5) + bitLengthForInt(mag[0]) - 1;
+
+// exponent == floor(log2(abs(this)))
+if (exponent Long.SIZE - 1) {
+return longValue();
+} else if (exponent Float.MAX_EXPONENT) {
+return signum 0 ? Float.POSITIVE_INFINITY :
Float.NEGATIVE_INFINITY;
+}
+
+/*
+ * We need the top SIGNIFICAND_BITS + 1 bits, including the implicit
+ * one bit. To make rounding easier, we pick out the top
+ * SIGNIFICAND_BITS + 2 bits, so we have one to help us round up or
+ * down. twiceSignifFloor will contain the top SIGNIFICAND_BITS + 2
+ * bits, and signifFloor the top SIGNIFICAND_BITS + 1.
+ *
+ * It helps to consider the real number signif = abs(this) *
+ * 2^(SIGNIFICAND_BITS - exponent).
+ */
+int shift = exponent - FloatConsts.SIGNIFICAND_WIDTH;
+
+int twiceSignifFloor;
+// twiceSignifFloor will be == abs().shiftRight(shift).intValue()
+// We do the shift into an int directly to improve performance.
+
+int nBits = shift 0x1f;
+int nBits2 = 32 - nBits;
+
+if (nBits == 0) {
+twiceSignifFloor = mag[0];
+} else {
+twiceSignifFloor = mag[0] nBits;
+if (twiceSignifFloor == 0) {
+twiceSignifFloor = (mag[0] nBits2) | (mag[1] nBits);
+}
+}
+
+int signifFloor = twiceSignifFloor 1;
+signifFloor = FloatConsts.SIGNIF_BIT_MASK; // remove the implied bit
+
+/*
+ * We round up if either the fractional part of signif is strictly
+ * greater than 0.5 (which is true if the 0.5 bit is set and any lower
+ * bit is set), or if the fractional part of signif is = 0.5 and
+ * signifFloor is odd (which is true if both the 0.5 bit and the 1 bit
+ * are set). This is equivalent to the desired HALF_EVEN rounding.
+ */
+boolean increment = (twiceSignifFloor 1) != 0
+ ((signifFloor 1) != 0 || abs().getLowestSetBit() shift);
+int signifRounded = increment ? signifFloor + 1 : signifFloor;
+int bits = ((exponent + FloatConsts.EXP_BIAS))
+ (FloatConsts.SIGNIFICAND_WIDTH - 1);
+bits += signifRounded;
+/*
+ * If signifRounded == 2^24, we'd need to set all of the significand
+ * bits to zero and add 1 to the exponent. This is exactly the behavior
+ * we get from just adding signifRounded to bits directly. If the
+ * exponent is Float.MAX_EXPONENT, we round up (correctly) to
+ * Float.POSITIVE_INFINITY.
+ */
+bits |= signum FloatConsts.SIGN_BIT_MASK;
+return Float.intBitsToFloat(bits);
}
/**
@@ -2986,8 +3053,80 @@
* @return this BigInteger converted to a {@code double}.
*/
public double doubleValue() {
-// Somewhat inefficient, but guaranteed to work.
Re: RFR 7131192: BigInteger.doubleValue() is depressingly slow
Would it be WIDTH or WIDTH - 1, i.e., with or without the implied bit? On Jun 17, 2013, at 11:32 AM, Louis Wasserman wrote: The comments mention SIGNIFICAND_BITS, which I think should probably be SIGNIFICAND_WIDTH?
Re: RFR 7131192: BigInteger.doubleValue() is depressingly slow
I'll update this but without reposting the webrev until I can upload the HTML version. On Jun 17, 2013, at 11:37 AM, Louis Wasserman wrote: You're right, it'd be WIDTH - 1, but since most of the comments refer to BITS + 1, that evens out nicely. On Mon, Jun 17, 2013 at 11:34 AM, Brian Burkhalterbrian.burkhal...@oracle.com wrote: Would it be WIDTH or WIDTH - 1, i.e., with or without the implied bit? On Jun 17, 2013, at 11:32 AM, Louis Wasserman wrote: The comments mention SIGNIFICAND_BITS, which I think should probably be SIGNIFICAND_WIDTH?
Re: RFR 623 build warnings [deprecation] isLoggable(int) in PlatformLogger
On 06/17/2013 05:15 PM, Mandy Chung wrote:
Chris,
Thanks for the patch. It looks fine to me. I saw that you have pushed
this changeset which is fine.
Thanks for looking at this Mandy, your eyes are welcome here.
As for the background, we leave these deprecated methods and fields in
sun.util.logging.PlatformLogger just temporarily to allow JavaFX to
migrate and use the PlatformLogger.Level enums [1] which has recently be
fixed.
Yes, I understand this, and the plan makes sense. I just notice these
warnings, and decided to clean up the jdk for now.
-Chris.
These deprecated methods/fields will be removed in JDK8:
JDK-8011638: Remove deprecated methods in
sun.util.logging.PlatformLogger
Mandy
[1] https://javafx-jira.kenai.com/browse/RT-29523
On 6/14/2013 8:00 AM, Chris Hegarty wrote:
There are 623 occurrences, and hence 623 deprecated build warnings, of
PlatformLogger.isLoggable(int) in the jdk source.
PlatformLogger is an internal API, and used in may places in the jdk.
isLoggable(int) has been deprecated in favor of isLoggable(Level).
isLoggable(Level) is slightly more efficient, as it avoids the mapping
of int to Level.
The solution is to simply replace isLoggable(int) with
isLoggable(Level), providing the appropriate Level. Example:
- if(logger.isLoggable(PlatformLogger.FINE)) {
+ if(logger.isLoggable(PlatformLogger.Level.FINE)) {
http://cr.openjdk.java.net/~chegar/platLoggerWarn/webrev/
I haven't yet filed a bug for this, but I plan to push the changes
through TL, rather than splitting the across multiple integration
forests.
Sample warning output:
tl/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java:416:
warning: [deprecation] isLoggable(int) in PlatformLogger has been
deprecated
tl/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java:422:
warning: [deprecation] isLoggable(int) in PlatformLogger has been
deprecated
tl/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java:635:
warning: [deprecation] isLoggable(int) in PlatformLogger has been
deprecated
-Chris.
Re: Inefficient code of String.indexOf(String)
Thank you all for the quick response. I will perform some tests and get in touch again asap. On Jun 17, 2013 10:45 PM, David Chase david.r.ch...@oracle.com wrote: For sufficiently large strings, indexOf can also be profitably parallelized. David On 2013-06-17, at 2:14 AM, Martin Buchholz marti...@google.com wrote: You are not the first person to have this idea. It is unlikely that you will succeed in changing the algorithm, because the jit-optimized brute-force algorithm is almost always faster. But go ahead and prove me wrong! On Sun, Jun 16, 2013 at 3:53 AM, Anubhav Chaturvedi mailforanub...@gmail.com wrote: Hello, I have recently started to explore the source code and am new to the open source community. I observed that in String.class within java.lang , the indexOf method, line 1715, uses the bruteforce approach when it comes to string matching. This method is used by the contains(CharSequence) method. There are a number of algorithms that can perform the task more efficiently. I would like to bring the required changes and needed your advice on this.
hg: jdk8/tl/jaxp: 8016133: Regression: diff. behavior with user-defined SAXParser
Changeset: 09d55894844d Author:joehw Date: 2013-06-17 12:47 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/09d55894844d 8016133: Regression: diff. behavior with user-defined SAXParser Reviewed-by: chegar, dfuchs ! src/org/xml/sax/helpers/XMLReaderFactory.java
Re: RFR 8015395: NumberFormatException during startup if java.lang.Integer.IntegerCache.high set to bad value
On Jun 12 2013, at 16:45 , Brian Burkhalter wrote: As no consensus was achieved, I am following up on this thread from last month http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/017493.html The behavior with and without the initially suggested patch http://cr.openjdk.java.net/~bpb/8015395/ for a few examples is listed below. As may be seen, the change suppresses the exception for bad values of the java.lang.Integer.IntegerCache.high property passed on the command line (silently falls back to the default value), but appears not to affect the behavior of the AutoBoxCacheMax property for these same values. There was more support expressed in the thread last month for throwing the NumberFormatExceotion rather than ignoring the bad value and falling back silently to the default. I tend to agree with that approach in general, but I think that the question here is really whether the same logic applies to a property such as java.lang.Integer.IntegerCache.high which is not publicly supported but rather intended to be used as an internal conduit. That the property is not publicly supported is important. The property is supposed to be set only by one user and we can adequately test that usage. Having started with the source I was unaware that the correct way to adjust the cache was to use the -XX:AutoBoxCacheMax option! Brian --- WITH Patch --- $ java -Xint -Djava.lang.Integer.IntegerCache.high=888 GoodbyeWorld Goodbye Cruel World! $ java -Xint -Djava.lang.Integer.IntegerCache.high=1024\-Xmx2g GoodbyeWorld Goodbye Cruel World! $ java -Xint -XX:AutoBoxCacheMax=888 GoodbyeWorld Goodbye Cruel World! $ java -Xint -XX:AutoBoxCacheMax=1024\-Xmx2g GoodbyeWorld Improperly specified VM option 'AutoBoxCacheMax=1024-Xmx2g' Error: Could not create the Java Virtual Machine. Error: A fatal exception has occurred. Program will exit I am OK with this outcome. I would still prefer to see an error for the AutoBoxCacheMax=888 case but can live with this result. Mike
Re: RFR: 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently
On 6/17/2013 10:36 AM, Daniel Fuchs wrote: On 6/17/13 7:11 PM, Mandy Chung wrote: FYI. Staffan has a fix in hotspot for JDK-8016304 the ThreadMXBean issue [1]. Thanks Mandy. Yes I saw that. I will wait for Staffan's fix - revert my changes, and run the test again - and see what I get. Perhaps you could pull his change into your local repo and verify if it resolves this issue. Mandy Alternatively I could now modify the test to try detect JDK-8016304 too and throw an exception pointing at 8016304 if the thread ids returned by findDeadlockedThread do not seem to indicate a deadlock. -- daniel Thanks Mandy [1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-June/010194.html On 6/17/2013 6:21 AM, Daniel Fuchs wrote: Hi, Please find below a webrev for fixing 8014218: test/java/util/logging/DrainFindDeadlockTest.java failing intermittently http://cr.openjdk.java.net/~dfuchs/JDK-8014218/webrev.00/ Without the fix - it's quite easy to make the test fail - especially if you copy paste the @run line a few times. This test is supposed to detect a deadlock in Logger initialization, but it detects too many false positive. An analysis of stack traces that the test dumps when it fails shows that there is actually no deadlock, since one of the two supposed deadlock threads is RUNNABLE. However - this - I think - indicates a bug in ThreadMXBean.findDeadlockedThreads(). I have rewritten the test to sanitize the results before and after calling ThreadMXBean.findDeadlockedThreads(). Namely - the test will call ThreadMXBean.findDeadlockedThreads() only if the two threads it monitors are simultaneously blocked (obtained by examining ThreadMXBean.getThreadInfo(long[])), and will verify that all threads found in deadlock by ThreadMXBean.findDeadlockedThreads() are indeed blocked. Now - I'm not sure whether this is a good thing or not: pros: it makes it possible to run this test again - and have a greater confidence that if it fails it's because of a genuine deadlock, which is desirable. cons: it 'hides' a possible bug in ThreadMXBean.findDeadlockedThreads() as it appears the test was testing ThreadMXBean.findDeadlockedThreads() more than (or as much as) it was testing the Logger implementation. Does it make sense to push this fix and log a bug against ThreadMXBean? Or should I instead plan to add this test to the problem list (and re-qualify the bug as an issue in ThreadMXBean)? guidance appreciated! best regards, -- daniel
hg: jdk8/tl/langtools: 8016779: Fix doclint warnings in javax.lang.model
Changeset: b7a10bc02e7a Author:darcy Date: 2013-06-17 14:46 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b7a10bc02e7a 8016779: Fix doclint warnings in javax.lang.model Reviewed-by: jjg ! src/share/classes/javax/lang/model/util/ElementScanner6.java ! src/share/classes/javax/lang/model/util/ElementScanner7.java ! src/share/classes/javax/lang/model/util/ElementScanner8.java ! src/share/classes/javax/lang/model/util/SimpleTypeVisitor6.java
Re: Code Review Request: 8016698: Cleanup overrides warning in sun/tools/ClassDeclaration.java
I spent some more time reviewing this, I believe the equals() implementation is correct. Given one string, we get a unique Identifier. The Identifier has the associated Type cached - so given an Identifier, we get a unique Type too. One usage of the ClassDeclaration#equals() method is in ClassDefinition#subClassOf(), which walks up the inheritance path of the concerned class to check if any of those are equal to the argument ClassDeclaration (and hence have equal Types).. - Kurchi On 6/17/2013 10:45 AM, Kurchi Hazra wrote: On 6/14/2013 11:56 PM, Alan Bateman wrote: On 14/06/2013 23:59, Kurchi Hazra wrote: Hi, This is to elimnate the overrides warning from ClassDeclaration.java. This class is used by rmi, but sun/tools/java and sun/tools/javac also heavily uses it, let me know if anyone else should also be reviewing it. Bug: http://bugs.sun.com/view_bug.do?bug_id=8016698 [To appear] Webrev: http://cr.openjdk.java.net/~khazra/8016698/webrev.00/ Thanks, Kurchi This looks okay for the purposes of squashing the warning but the existing equals method does look suspect. It might not matter for the rmic usage but as the equals doesn't take account of the the definition then it looks to me that it consider very different ClassDeclarations as equals. It would be good to understand how it is used to see whether we have an issue here or not. Thanks for the review. If you analyse definition, it is not constant and changes as the state of the class changes (whether binary class is loaded, whether it has been compiled and so on) - and so does the status field. The type field looks like the only good candidate to use in the equals method - so the equals() looks correct to me. Now the question is whether there is any need of overriding Object.equals() at all, that is, whether each ClassDeclaration object should be treated as a unique one - I am looking around in the source code to check that. Thanks, - Kurchi
hg: jdk8/tl/jdk: 7177472: JSR292: MethodType interning penalizes scalability
Changeset: 2b63fda275a3 Author:twisti Date: 2013-06-17 16:24 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2b63fda275a3 7177472: JSR292: MethodType interning penalizes scalability Reviewed-by: twisti Contributed-by: Aleksey Shipilev aleksey.shipi...@oracle.com ! src/share/classes/java/lang/invoke/MethodType.java
Re: RFR : 8016446 : (m) Add override forEach/replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap
On Jun 17 2013, at 03:09 , Paul Sandoz wrote:
On Jun 14, 2013, at 11:57 PM, Mike Duigou mike.dui...@oracle.com wrote:
I have updated the webrev again. In addition to moving the modification
checks to be consistently after the operation for the most complete
fast-fail behaviour I've also slightly enhanced the Map default to detect
ISE thrown by setValue as a CME condition.
http://cr.openjdk.java.net/~mduigou/JDK-8016446/2/webrev/
There are some places where the indentation looks wonky e.g. HashMap
LinkedHashMap changes.
I have restyled all of the diffs to make sure they are clean.
The LinkedHashMap.forEach/replaceAll methods are checking modCount and
throwing CME before the action is called.
Corrected.
The WeakHashMap.replaceAll method is checking the modCount per bucket,
not-per element.
Corrected.
Are there existing tests for the overriding methods?
I had believed so in Map/Defaults.java but replaceAll was absent. Now corrected
in updated webrev
http://cr.openjdk.java.net/~mduigou/JDK-8016446/3/webrev/
I had to add the improved default for ConcurrentMap which was present in the
lambda repo in order to have correct behaviour. Since getOrDefault is already
in ConcurrentMap I will include this but we have to be careful when we do a jsr
166 syncup to make sure that the defaults don't get lost.
Mike
Otherwise looks OK.
Paul.
For interference detection the strategy I am advocating is :
- serial non-stream operations
(Collection.forEach/Iterator.forEachRemaining): per-element post-operation
(fast-fail, best-efforts). As Remi has pointed out the failure modes for
collection.forEach() and for(:collection) will differ and it is a conscious
choice to provide superior fast-fail than to match behaviour.
- stream terminal operations, both serial and parallel : at completion if at
all. Having the serial and parallel stream interference detection behaviours
consistent is prioritized over making serial stream behaviour match
behaviour of non-stream iteration methods.
Mike
On Jun 12 2013, at 22:28 , Mike Duigou wrote:
I have updated my webrev with Remi's improvements and some other
improvements to the fast-fail concurrent modification checking.
Revised webrev:
http://cr.openjdk.java.net/~mduigou/JDK-8016446/1/webrev/
Mike
On Jun 12 2013, at 15:49 , Remi Forax wrote:
On 06/12/2013 11:23 PM, Mike Duigou wrote:
Hello all;
This patch adds optimized implementations of Map.forEach and
Map.replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap,
TreeMap
http://cr.openjdk.java.net/~mduigou/JDK-8016446/0/webrev/
The implementations traverse the map more efficiently than the default
iterator implementation and sometimes avoid creation of transient
Map.Entry instances. The fast-fail behaviour of the default
implementation is retained.
Mike
Hi Mike,
funnily I was writing HashMap.forEach/LinkedHashMap.forEach at the same
time.
(you need also to override forEach in LinkedHashMap because the one you
inherits from HashMap doesn't use the linked list of entries).
My code is slightly different from yours because I've moved the cases
where the item is a red/black tree out of the fast path
(the tree is created either if you are unlucky, if hashCode is badly
written or if someone forge keys to have collisions)
and does the modCount check for each element because a call to the
consumer can change the underlying map
so you can not do a modCount check only at the end.
Rémi
Here is the diff.
diff -r 6df79b7bae6f src/share/classes/java/util/HashMap.java
--- a/src/share/classes/java/util/HashMap.javaWed Jun 12 09:44:34 2013
+0100
+++ b/src/share/classes/java/util/HashMap.javaThu Jun 13 00:46:05 2013
+0200
@@ -28,6 +28,7 @@
import java.io.*;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
+import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.BiFunction;
import java.util.function.Function;
@@ -2615,6 +2616,54 @@
int capacity() { return table.length; }
float loadFactor() { return loadFactor; }
+
+@Override
+public void forEach(BiConsumer? super K, ? super V action) {
+Objects.requireNonNull(action);
+final int expectedModCount = modCount;
+if (nullKeyEntry != null) {
+forEachNullKey(expectedModCount, action);
+}
+Object[] table = this.table;
+for(int index = 0; index table.length; index++) {
+Object item = table[index];
+if (item == null) {
+continue;
+}
+if (item instanceof HashMap.TreeBin) {
+eachTreeNode(expectedModCount, ((TreeBin)item).first,
action);
+continue;
+}
+@SuppressWarnings(unchecked)
+EntryK,V entry = (EntryK,V)item;
+for (;entry
hg: jdk8/tl/jdk: 8014620: Signature.getAlgorithm return null in special case
Changeset: 116050227ee9 Author:youdwei Date: 2013-06-17 17:36 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/116050227ee9 8014620: Signature.getAlgorithm return null in special case Reviewed-by: wetmore ! src/share/classes/java/security/Signature.java + test/java/security/Signature/SignatureGetAlgorithm.java
Re: RFR (XS) CR 8014233: java.lang.Thread should have @Contended on TLR fields
Hi Aleksey, What is the overall change in memory use for this set of changes ie what did we use pre TLR merging and what do we use now? Thanks, David On 17/06/2013 7:00 PM, Aleksey Shipilev wrote: Hi, This is the respin of the RFE filed a month ago: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016754.html The webrev is here: http://cr.openjdk.java.net/~shade/8014233/webrev.02/ Testing: - JPRT build passes - Linux x86_64/release passes jdk/java/lang jtreg - vm.quick.testlist, vm.quick-gc.testlist on selected platforms - microbenchmarks, see below The rationale follows. After we merged ThreadLocalRandom state in the thread, we are now missing the padding to prevent false sharing on those heavily-updated fields. While the Thread is already large enough to separate two TLR states for two distinct threads, we can still get the false sharing with other thread fields. There is the benchmark showcasing this: http://cr.openjdk.java.net/~shade/8014233/threadbench.zip There are two test cases: first one is only calling its own TLR with nextInt() and then the current thread's ID, another test calls *another* thread ID, thus inducing the false sharing against another thread's TLR state. On my 2x2 i5 laptop, running Linux x86_64: same:355 +- 1 ops/usec other: 100 +- 5 ops/usec Note the decrease in throughput because of the false sharing. With the patch: same:359 +- 1 ops/usec other: 356 +- 1 ops/usec Note the performance is back. We want to evade these spurious decreases in performance, due to either unlucky memory layout, or the user code (un)intentionally ruining the cache line locality for the updater thread. Thanks, -Aleksey.
hg: jdk8/tl/langtools: 8013789: Compiler should emit bridges in interfaces
Changeset: 455be95bd1b5 Author:rfield Date: 2013-06-17 20:29 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/455be95bd1b5 8013789: Compiler should emit bridges in interfaces Summary: paired with 8015402: Lambda metafactory should not attempt to determine bridge methods Reviewed-by: vromero Contributed-by: maurizio.cimadam...@oracle.com ! src/share/classes/com/sun/tools/javac/code/Types.java ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/comp/Check.java ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java ! src/share/classes/com/sun/tools/javac/comp/TransTypes.java ! src/share/classes/com/sun/tools/javac/tree/JCTree.java ! test/tools/javac/lambda/lambdaExpression/LambdaTest6.java ! test/tools/javac/lambda/methodReference/BridgeMethod.java
