Re: RFR: 8284853: Fix varios 'expected' typo
On Wed, 13 Apr 2022 20:36:48 GMT, Andrey Turbanov wrote:
> Found various typos of expected: `exepected`, `exept`, `epectedly`,
> `expeced`, `Unexpeted`, etc.
test/jdk/java/lang/StackWalker/StackStreamTest.java line 218:
> 216: private static void equalsOrThrow(String label, List list,
> List expected) {
> 217: System.out.println("List: " + list);
> 218: System.out.println("Expected: " + list);
I think there is a per-existing bug here. Shouldn't the second println print
`expected` instead of `list`?
-
PR: https://git.openjdk.java.net/jdk/pull/8231
Re: RFR: 8274716: JDWP Spec: the description for the Dispose command confuses suspend with resume.
On Mon, 4 Oct 2021 13:44:44 GMT, Richard Reingruber wrote: > The following sentence in the JDWP Specification describing the Dispose > command confuses resume with suspend [1]: > > All threads suspended by the thread-level **resume** command or the VM-level > **resume** command are resumed as many times as necessary for them to run. > > It should be changed to > > All threads suspended by the thread-level **suspend** command or the > VM-level > **suspend** command are resumed as many times as necessary for them to run. > > [1] [JDWP Spec, Dispose Command > (JDK17).](https://docs.oracle.com/en/java/javase/17/docs/specs/jdwp/jdwp-protocol.html#JDWP_VirtualMachine_Dispose) Can you update the copyright please? I checked the JDI spec and it looks correct there, which is actually surprising since errors like this usually appear in both specs. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5804
Re: RFR: 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m [v2]
On Wed, 3 Feb 2021 20:17:03 GMT, Phil Race wrote: >> This removes the JNF dependency from the jdk.hotspot.agent module. >> The macro expansions are the same as already used in the Java desktop module >> - which actually uses a macro >> still but there there are hundreds of uses. >> The function of this macro code is to prevent NSExceptions escaping to Java >> and also to drain the auto-release pool. >> What test group would be good for verifying this change ? > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2304
Re: RFR: 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m
On Thu, 28 Jan 2021 22:40:57 GMT, Phil Race wrote:
> This removes the JNF dependency from the jdk.hotspot.agent module.
> The macro expansions are the same as already used in the Java desktop module
> - which actually uses a macro
> still but there there are hundreds of uses.
> The function of this macro code is to prevent NSExceptions escaping to Java
> and also to drain the auto-release pool.
> What test group would be good for verifying this change ?
src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 294:
> 292:
> 293: NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
> 294: @try {
Although there are only 3 places where the `JNF_COCOA_ENTER/EXIT` macros are
used, it seems it would still be worth taking the same approach you did for
`java.desktop` and add the replacement macros instead of inlining them. So just
copy what you added to
`src/java.desktop/macosx/native/libosxapp/JNIUtilities.h`, and replace
`JNF_COCOA_ENTER` with `JNI_COCOA_ENTER/EXIT`. Otherwise the
`JNF_COCOA_ENTER/EXIT` changes look fine to me, but I'm just basing this on a
comparison with what you've done with `java.desktop`. I'm no expert in this
area.
src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 296:
> 294: @try {
> 295:
> 296: NSString *symbolNameString = JavaStringToNSString(env, symbolName);
Is there a reason why `java.desktop` continues to use `JNFJavaToNSString`? I
was looking to see how this was handled in other places, but I couldn't find an
example where `JNFJavaToNSString` was converted to call a newly implemented
`JavaStringToNSString`.
-
PR: https://git.openjdk.java.net/jdk/pull/2304
Re: RFR: 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m
On Fri, 29 Jan 2021 00:15:56 GMT, Chris Plummer wrote: >> This removes the JNF dependency from the jdk.hotspot.agent module. >> The macro expansions are the same as already used in the Java desktop module >> - which actually uses a macro >> still but there there are hundreds of uses. >> The function of this macro code is to prevent NSExceptions escaping to Java >> and also to drain the auto-release pool. >> What test group would be good for verifying this change ? > > For testing you want `test/jdk/sun/tools/jhsdb/` and > `test/hotspot/jtreg/serviceability/sa` I'm doubtful you'll find anyone on serviceability-dev that understands JNF and the implications it has on MacosxDebuggerLocal.m. Although I've done a lot of work in this file myself recently, it's all bee sans any knowledge of JNF, Cocoa, or Objective C. You might be better off asking reviewers that looked at other recent PRs to remove JNF usage. However, having looked through [JDK-8257852](https://bugs.openjdk.java.net/browse/JDK-8257852) and from there finding [JDK-8259651](https://bugs.openjdk.java.net/browse/JDK-8259651), I'm wondering why you didn't just replace JNF_COCOA_ENTER/EXIT with the new JNI_COCOA_ENTER/EXIT in this PR also? Is it because they are not in a place that can be accessed from this file? - PR: https://git.openjdk.java.net/jdk/pull/2304
Re: RFR: 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m
On Thu, 28 Jan 2021 22:40:57 GMT, Phil Race wrote: > This removes the JNF dependency from the jdk.hotspot.agent module. > The macro expansions are the same as already used in the Java desktop module > - which actually uses a macro > still but there there are hundreds of uses. > The function of this macro code is to prevent NSExceptions escaping to Java > and also to drain the auto-release pool. > What test group would be good for verifying this change ? For testing you want `test/jdk/sun/tools/jhsdb/` and `test/hotspot/jtreg/serviceability/sa` - PR: https://git.openjdk.java.net/jdk/pull/2304
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 19:38:16 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with two additional > commits since the last revision: > > - Refactor CDS disabling > - Redo builsys support for aarch64-darwin JDI changes also look good. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 19:38:16 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with two additional > commits since the last revision: > > - Refactor CDS disabling > - Redo builsys support for aarch64-darwin I looked through the SA changes. Overall looks good except for a couple of minor issues noted. For the most part it seems to have been boilerplate copy-n-paste from existing ports. If there is anything you want me to take a closer look at, let me know. src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 702: > 700: primitiveArray = (*env)->GetLongArrayElements(env, registerArray, > NULL); > 701: > 702: #undef REG_INDEX I'm not so sure why the #undef and subsequent #define of REG_INDEX is needed since it seems to just get #define'd back to the same value. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/aarch64/BsdAARCH64ThreadContext.java line 2: > 1: /* > 2: * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. Update copyright. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 02:40:29 GMT, Yasumasa Suenaga wrote: >> Marked as reviewed by iklam (Reviewer). > > @iklam Thanks for your review! > >> I looked at a cases in the JDK code where >> `Java_sun_security_pkcs11_wrapper_PKCS11_connect()` calls `FormatMessage()`. >> It eventually passes the `char*` to `jni_ThrowNew`, which eventually gets to >> `Exceptions::new_exception` with `to_utf8_safe==safe_to_utf8`. This means >> the message will be converted with `java_lang_String::create_from_str()`, >> which does NOT call `JNU_NewStringPlatform`. So I think in this case, the >> error message will also be garbled. >> >> java.base/windows/native/libnet/Inet4AddressImpl.c seems to have a similar >> problem in the `ping4` function. > > Agree. `ping4()` calls `NET_ThrowNew()` which passes `ThrowNew()` JNI > function without modifying arguments. > I looked at other cases in JDK code, following places has possible to garble: > > * `throwByName()` @ jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c > * `Java_sun_security_pkcs11_Secmod_nssLoadLibrary()` @ j2secmod_md.c > * jdk.jdi/share/native/libdt_shmem/SharedMemoryTransport.c > * `throwShmemException()` @ SharedMemoryTransport.c > >> But, I don't know how to write a simple test case for the above. > > Agree, we might need to fix garbled messages one by one when we find out > issues. > IMHO we need to establish rule that we have to use `JNU_NewStringPlatform()` > if we want to throw exception with message from platform (`FormatMessage()`, > `strerror()`) from JNI. Given that this seems to be a common problem in our code, and likely a very very old problem at that, why has it never been reported before? I'm not questioning the fix except to the extent that I'm questioning our understanding of the problem. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 00:41:11 GMT, Yasumasa Suenaga wrote: > jdk.hotspot.agent do not have `FormatMessage()` call in other place. > Did you say about whole JDK code? I haven't checked all of them, but some > code (e.g. net_util_md.c) uses `JNU_ThrowByName()` which is provided by > java.dll. Yes, I was referring to all JDK code. Given how many references there are to FormatMessage(), I'm surprised this issue has not turned up before. I was wondering if there is something unique about the SA reference that makes the issue only turn up there. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. There are probably 25 or so places in our code where we use FormatMessage to get the error message. Are these all going to run into the same FormateMessage bug? Also, it's not clear to me why you are getting garbled text in the first place. You said "Windows API might not return UTF-8 encoded string on Japanese locale." Why is that the case? src/jdk.hotspot.agent/share/native/libsaproc/sadis.c line 75: > 73: > 74: #ifdef _WINDOWS > 75: static int getLastErrorString(char *buf, size_t len) Is this being removed because a copy already exists in jni_util_md.c, and you now have access to this copy because you are linking against java.dll? - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8257928: Test image build failure with clang-10 due to -Wmisleading-indentation
On Wed, 9 Dec 2020 07:11:53 GMT, Hao Sun
wrote:
> Flag '-Wmisleading-indentation' was introduced since clang-10 [1] and
> gcc-6 [2]. Putting the code with proper indentations would suppress this
> warning.
>
> The main reason why test image build with gcc succeeds is that, clang
> and gcc might behave differently for some corner cases under
> '-Wmisleading-indentation'.
>
> The following code snippet is crafted based on this build failure, where
> each statement with improper indentation (line A and line B) follows an
> if-statement. The key difference between foo() and bar() lies in that
> there exists an extra comment, i.e. "/* comment 2 */", between statement
> at line A and the if-statement.
>
> int foo(int num) {
> /* comment 1 */
> if (num > 1)
> return 0;
>
> /* comment 2 */
> num = num + 1; // line A
> return num;
> }
>
> int bar(int val) {
> /* comment 3 */
> if (val > 1)
> return 0;
>
> val = val + 1; // line B
> return val;
> }
>
> It's worth noting that foo() is quite similar to the erroneous code in
> this build failure. Clang-10 would emit misleading indentation warnings
> for both foo() and bar(), whereas gcc-6 or higher considers foo() free
> of problems. [3]
>
> This patch is a complement to JDK-8253892. In JDK-8253892, flag
> '-Wmisleading-indentation' is disabled for both clang and gcc when
> building JRE and JDK images. This patch does not disable the flag for
> test image building, but just fixes the code, becasue:
> - only a small number of warnings are produced and it's easy to fix
> them one by one.
> - inconsistent warning behaviors between gcc and clang are triggered
> by this case and simply disabling the flag does not work.
>
> Note that AArch64 is NOT tested because test image build still fails
> after this bug is fixed due to another runtime error (See JDK-8257936),
> which is not related to this patch.
>
> [1]
> https://releases.llvm.org/10.0.0/tools/clang/docs/DiagnosticsReference.html
> [2]
> https://gcc.gnu.org/onlinedocs/gcc-6.5.0/gcc/Warning-Options.html#Warning-Options
> [3] https://godbolt.org/z/xs6xWv
>
> -
> We have tested with this patch, the test image can be built successfully with
> clang-10 on Linux X86 machine.
Marked as reviewed by cjplummer (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/1709
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v12]
On Tue, 29 Sep 2020 14:08:49 GMT, Monica Beckwith wrote: >> This is a continuation of >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html >> >> Changes since then: >> * We've improved the write barrier as suggested by Andrew [1] >> * The define-guards around R18 have been changed to `R18_RESERVED`. This >> will be enabled for Windows only for now but >> will be required for the upcoming macOS+Aarch64 [2] port as well. >> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov >> in our PR for now and built the >> Windows-specific CPU feature detection on top of it. >> >> [1] >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html >> [2] https://openjdk.java.net/jeps/8251280 > > Monica Beckwith has updated the pull request incrementally with one > additional commit since the last revision: > > change string representation for r18 to "r18_tls" on every platform Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v7]
On Tue, 29 Sep 2020 14:03:57 GMT, Bernhard Urban-Forster wrote: >> Marked as reviewed by aph (Reviewer). > > @theRealAph okay, I've changed the string representation of `r18` to > `"r18_tls"` on every platform. > @plummercj thank you for your feedback. I've updated the copyright in > mentioned files. Changes look good. Thanks. I'm marking as "reviewed" for serviceability changes and copyrights. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v6]
On Thu, 24 Sep 2020 14:04:22 GMT, Monica Beckwith wrote: >> This is a continuation of >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html >> >> Changes since then: >> * We've improved the write barrier as suggested by Andrew [1] >> * The define-guards around R18 have been changed to `R18_RESERVED`. This >> will be enabled for Windows only for now but >> will be required for the upcoming macOS+Aarch64 [2] port as well. >> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov >> in our PR for now and built the >> Windows-specific CPU feature detection on top of it. >> >> [1] >> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html >> [2] https://openjdk.java.net/jeps/8251280 > > Monica Beckwith has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev > excludes the unrelated changes brought in by the merge/rebase. The pull > request contains 17 additional commits since > the last revision: > - cleanup for 8253539: Remove unused JavaThread functions for > set_last_Java_fp/pc > - cleanup for 8253457: Remove unimplemented register stack functions > - Merge remote-tracking branch 'upstream/master' into jdk-windows > - Update orderAccess_windows_aarch64.hpp > >changing from Acq-reL to Sequential Consistency to avoid compiler > reordering when no ordering hints are provided > - 8248787: G1: Workaround MSVC bug >Reviewed-by: >Contributed-by: mbeckwit, luhenry, burban > - 8248670: Windows: Exception handling support on AArch64 >Reviewed-by: >Contributed-by: mbeckwit, luhenry, burban > - 8248660: AArch64: Make _clear_cache and _nop portable >Summary: __builtin___clear_cache, etc. >Contributed-by: mbeckwit, luhenry, burban > - 8248659: AArch64: Extend CPU Feature detection >Reviewed-by: >Contributed-by: mbeckwit, luhenry, burban > - 8248656: Add Windows AArch64 platform support code >Reviewed-by: >Contributed-by: mbeckwit, luhenry, burban > - 8248498: Add build system support for Windows AArch64 >Reviewed-by: >Contributed-by: mbeckwit, luhenry, burban > - ... and 7 more: > https://git.openjdk.java.net/jdk/compare/ddd43bee...2b662010 I looked at changes to existing SA files. These changes look fine. I did not look at the new aarch64 SA files other than the copyright section. I assume they are clones of the x64 versions with some symbolic renaming. If there is any more than that and you'd like me to have a look, let me know. As for the copyright in the new SA files, I believe it is incorrect and needs to include Oracle. There are a number of other non-SA files that are new and also have the same copyright issue. I also looked at src/jdk.attach/windows/classes/sun/tools/attach/AttachProviderImpl.java. It looks fine except it needs a copyright date update. - Changes requested by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: JDK-8242629 Remove references to deprecated java.util.Observer and Observable
Hi Magnus, Some minor mistakes below, but otherwise looks good. Also copyrights need updating. I don't need to see another webrev. thanks, Chris --- old/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/MethodHandlesAdapterBlob.java 2020-04-14 12:47:05.098156117 +0200 +++ new/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/MethodHandlesAdapterBlob.java 2020-04-14 12:47:04.774156119 +0200 @@ -28,6 +28,10 @@ import sun.jvm.hotspot.debugger.*; import sun.jvm.hotspot.runtime.*; import sun.jvm.hotspot.types.*; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; --- old/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java 2020-04-14 12:48:16.314155591 +0200 +++ new/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java 2020-04-14 12:48:15.962155594 +0200 @@ -33,6 +33,12 @@ import sun.jvm.hotspot.runtime.*; import sun.jvm.hotspot.types.*; import sun.jvm.hotspot.utilities.*; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; --- old/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/GenericGrowableArray.java 2020-04-14 12:49:58.998154834 +0200 +++ new/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/GenericGrowableArray.java 2020-04-14 12:49:58.674154836 +0200 @@ -29,6 +29,10 @@ import sun.jvm.hotspot.runtime.*; import sun.jvm.hotspot.oops.*; import sun.jvm.hotspot.types.*; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; +import sun.jvm.hotspot.utilities.Observable; +import sun.jvm.hotspot.utilities.Observer; +++ new/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/Observable.java 2020-04-14 12:50:10.634154748 +0200 ... \ No newline at end of file On 4/14/20 4:04 AM, Magnus Ihse Bursie wrote: As a first step towards fixing deprecation warnings in SA, all the references (200+) to the deprecated java.util.Observer and Observable needs to be fixed, otherwise all other changes will drown in this one. This solution is the result of the preceding discussions in serviceability-dev. That means that most of the change consists of adding an explicit import like this: import sun.jvm.hotspot.utilities.Observable; import sun.jvm.hotspot.utilities.Observer; to override the general java.util.* import that was already present in all (or almost all) files, and make sun.jvm.hotspot.utilities.Observable and Observer extend the java.util versions but with deprecation warnings disabled. It turned out however, that this simplest approach did not work fully. Since the interface java.util.Observer had the single method "void update(java.util.Observable o, Object arg)" it did not help to create a new interface sun.jvm.hotspot.utilities.Observer that extended java.util.Observer. I did not observe this issue in my PoC webrev that I posted during the discussion. :-( Instead, for Observer, I had just created a new interface with the same method, but that uses sun.jvm.hotspot.utilities.Observable instead of java.util.Observable. The end effect is the same -- the only change needed to most files is an added import, we get rid of the deprecation warnings, and we did not have to copy any significant amount of code from java.util. I now hope this is acceptable by all. Bug: https://bugs.openjdk.java.net/browse/JDK-8242629 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8242629-fix-SA-Observer/webrev.01 (When reading the patch, I recommend looking at the patch file http://cr.openjdk.java.net/~ihse/JDK-8242629-fix-SA-Observer/webrev.01/open.patch instead of individually checking the files in the webrev.) /Magnus
Re: RFR: JDK-8241618 Fix unchecked warning for jdk.hotspot.agent
Hi Magus,
I haven't looked at the changes yet, other to see that there are many
files touched, but after reading below (and only partly understanding
since I don't know this area well), I was wondering if this issue
wouldn't be better served with multiple passes made to fix the warnings.
Start with a straight forward one where you are maybe only making one or
two types of changes, but that affect a large number of files and don't
cascade into other more complicated changes. This will get a lot of the
noise out of the way, and then we can focus on some of the harder issues
you bring up below.
As for testing, I think the following list will capture all of them, but
can't say for sure:
open/test/hotspot/jtreg/serviceability/sa
open/test/hotspot/jtreg/resourcehogs/serviceability/sa
open/test/jdk/sun/tools/jhsdb
open/test/jdk/sun/tools/jstack
open/test/jdk/sun/tools/jmap
open/test/hotspot/jtreg/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java
open/test/hotspot/jtreg/compiler/ciReplay/TestSAClient.java
open/test/hotspot/jtreg/compiler/ciReplay/TestSAServer.java
Chris
On 3/25/20 12:29 PM, Magnus Ihse Bursie wrote:
With the recent fixes in JDK-8241310, JDK-8237746 and JDK-8241073, and
the upcoming fixes to remove the deprecated nashorn and jdk.rmi, the
JDK build is very close to producing no warnings when compiling the
Java classes.
The one remaining sinner is jdk.hotspot.agent. Most of the warnings
here are turned off, but unchecked and deprecation cannot be
completely silenced.
Since the poor agent does not seem to receive much love nowadays, I
took it upon myself to fix these warnings, so we can finally get a
quiet build.
I started to address the unchecked warnings. Unfortunately, this was a
much bigger task than I anticipated. I had to generify most of the
module. On the plus side, the code is so much better now. And most of
the changes were trivial, just tedious.
There are a few places were I'm not entirely happy with the current
solution, and that at least merits some discussion.
I have resorted to @SuppressWarnings in four classes: ciMethodData,
MethodData, TableModelComparator and VirtualBaseConstructor. All of
them has in common that they are doing slightly fishy things with
classes in collections. I'm not entirely sure they are bug-free, but
this patch leaves the behavior untouched. I did some efforts to sort
out the logic, but it turned out to be too hairy for me to fix, and it
will probably require more substantial changes to the workings of the
code.
To make the code valid, I have moved ConstMethod to extend Metadata
instead of VMObject. My understanding is that this is benign (and
likely intended), but I really need for someone who knows the code to
confirm this. I have also added a FIXME to signal this. I'll remove
the FIXME as soon as I get confirmation that this is OK.
(The reason for this is the following piece of code from
Metadata.java: metadataConstructor.addMapping("ConstMethod",
ConstMethod.class))
In ObjectListPanel, there is some code that screams "dead" with this
change. I added a FIXME to point this out:
for (Iterator iter = elements.iterator(); iter.hasNext(); ) {
if (iter.next() instanceof Array) {
// FIXME: Does not seem possible to happen
hasArrays = true;
return;
}
It seems that if you start pulling this thread, even more dead code
will unravel, so I'm not so eager to touch this in the current patch.
But I can remove the FIXME if you want.
My first iteration of this patch tried to generify the IntervalTree
and related class hierarchy. However, this turned out to be impossible
due to some weird usage in AnnotatedMemoryPanel, where there seemed to
be confusion as to whether the tree stored Annotations or Addresses.
I'm not entirely convinced the code is correct, it certainly looked
and smelled very fishy. However, I reverted these changes since I
could not get them to work due to this, and it was not needed for the
goal of just getting rid of the warning.
Finally, I have done no testing apart from verifying that it builds.
Please advice on suitable tests to run.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241618
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8241618-fix-unchecked-warnings-for-agent/webrev.01
/Magnus
Re: [PING] RFR(XS): 8208091: SA: jhsdb jstack --mixed throws UnmappedAddressException on i686
Hi Severin, The changes look fine. Thanks for adding the test case. Chris On 8/6/18 4:53 AM, Severin Gehwolf wrote: Hi, Latest webrev with JNI properly compiled with -fomit-frame-pointer and -O3. There was a bug in the test where the exception wasn't properly rethrown: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8208091/webrev.03/ I'm going to run this through jdk-submit too. Adding build-dev for build changes. Thanks, Severin On Fri, 2018-08-03 at 18:45 +0200, Severin Gehwolf wrote: Hi all, On Mon, 2018-07-30 at 21:23 -0700, Sharath Ballal wrote: Ok. It looks like we don't even have a "jstack --mixed" test. Could you add one? It would be even better if the test included a JNI lib that wasn't compiled with -fno-omit-frame-pointer so you don't need to rely on glibc to reproduce this issue (or is glibc pretty much always compiled without -fno-omit-frame-pointer)? Or if Sharath agrees, file a bug to have a test added. That’s a good suggestion. Severin you can either write a test or open a bug for it. Latest webrev with a test: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8208091/webrev.02/ The test fails prior the test on affected systems and passes after. There are still issues with getting the test's JNI properly compiled the way it's supposed to. I've asked for help on build-dev[1]. Example runs: Before patch: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8208091/before_patch.txt After patch: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8208091/after_patch.txt Thanks, Severin [1] http://mail.openjdk.java.net/pipermail/build-dev/2018-August/022819.html -Original Message----- From: Chris Plummer Sent: Monday, July 30, 2018 10:03 PM To: Severin Gehwolf; Sharath Ballal; serviceability-dev Subject: Re: [PING] RFR(XS): 8208091: SA: jhsdb jstack --mixed throws UnmappedAddressException on i686 Hi Severin, On 7/30/18 1:28 AM, Severin Gehwolf wrote: Hi Chris, On Thu, 2018-07-26 at 14:07 -0700, Chris Plummer wrote: I had looked at this review when it came out, but was hesitant to ok it because I really don't know this code at all. If you can get another reviewer who does know the code, then I'll approve it. Sharath Ballal reviewed it, but he's not a Reviewer as per the OpenJDK census. As to whether he knows the code, I don't know. He's on CC. Yes, but I was asking for a second reviewer (not counting me). This only impacts 32-bit, right? If so, keep in mind that it won't get tested by Oracle testing, including the submit repo, so make sure you do thorough testing. It only impacts 32-bit, yes. I understand that Oracle isn't testing 32- bit x86 any more. The change itself should be fairly low risk since it's changing only a 32-bit-x86-linux-only file and the native bits don't seem to match what the Java code does[1]. REG_INDEX(reg) being defined as: #define REG_INDEX(reg) sun_jvm_hotspot_debugger_x86_X86ThreadContext_##reg and being used as: REG_INDEX(SP) Thus, using sun_jvm_hotspot_debugger_x86_X86ThreadContext_SP The Java code uses: sun.jvm.hotspot.debugger.x86.X86ThreadContext.ESP Also, why is there any code being executed that was not compiled with -fno-omit-frame-pointer? The description in the CR just shows a simple java program reproducing this, so all the mixed stack traces belong to the JVM and libs, and I thought we made sure to compile all of them with -fno-omit-frame-pointer. The JVM uses glibc and that simple program is enough to see some thread's stack currently being in a glibc function when getting a mixed stack trace. We've originally seen this in JDK 8 with jstack -m and was reported in [2]. That comment has more details. The problem here isn't that it's a JDK lib which gets compiled without -fno-omit-frame- pointer. It's glibc not being compiled with that option. An example stack trace for a system where this happens looks like this: Thread 7 (Thread 0xa3863b40 (LWP 834)): #0 0xf771f430 in __kernel_vsyscall () #1 0xf7703acc in futex_abstimed_wait (cancel=true, private=, abstime=0x0, expected=1, futex=0xf770f000) at ../nptl/sysdeps/unix/sysv/linux/sem_waitcommon.c:43 #2 do_futex_wait (sem=0xf770f000, sem@entry=0xf70ea854 , abstime=0x0) at ../nptl/sysdeps/unix/sysv/linux/sem_waitcommon.c:226 #3 0xf7703bb7 in __new_sem_wait_slow (sem=0xf70ea854 , abstime=0x0) at ../nptl/sysdeps/unix/sysv/linux/sem_waitcommon.c:407 #4 0xf6cc18d4 in check_pending_signals (wait=true) at /usr/src/debug/java-1.8.0-openjdk-1.8.0.171- 8.b10.el7_5.i386/openjdk/h otspot/src/os/linux/vm/os_linux.cpp:2522 #5 0xf6cbc632 in signal_thread_entry (thread=0xa37a4800, __the_thread__=0xa37a4800) at /usr/src/debug/java-1.8.0-openjdk-1.8.0.171- 8.b10.el7_5.i386/openjdk/h otspot/src/share/vm/runtime/os.cpp:250 That is, frames 0-3 are JDK foreign. This bug will happen on all systems which use any native library which isn't compiled with -fno- omit-frame-pointer. Be it glibc or some other library. Ok. It looks like we don't even have a "jstack --mixed"
RFR(S): 8196992: Resolve disabled warnings for libdt_socket
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8196992 diff --git a/make/lib/Lib-jdk.jdwp.agent.gmk b/make/lib/Lib-jdk.jdwp.agent.gmk --- a/make/lib/Lib-jdk.jdwp.agent.gmk +++ b/make/lib/Lib-jdk.jdwp.agent.gmk @@ -43,7 +43,6 @@ OPTIMIZATION := LOW, \ CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \ $(LIBDT_SOCKET_CPPFLAGS), \ - DISABLED_WARNINGS_gcc := shift-negative-value, \ MAPFILE := $(TOPDIR)/make/mapfiles/libdt_socket/mapfile-vers, \ LDFLAGS := $(LDFLAGS_JDKLIB) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ This change is undoing the makefile change done as part of JDK-8196985. The only warning that was turning up in libdt_socket code before JDK-8196985 was done has already been fixed by JDK-8196909. Thus no warnings need to be fixed. After removing the above makefile code, I tested by building with the new toolchain. As a first test I undid the socketTransport.cpp fix from JDK-8196909 to verify that the new toolchain exposed the warning. Then I reverted socketTransport.cpp back to tip sources and saw no warnings with the new toolchain. thanks, Chris
Re: PPC64: Poor StrictMath performance due to non-optimized compilation
On 11/21/16 4:27 PM, Gustavo Romero wrote:
Hi Joe,
On 17-11-2016 19:33, joe darcy wrote:
Currently, optimization for building fdlibm is disabled, except for the
"solaris" OS target [1].
The reason for that is because historically the Solaris compilers have had
sufficient discipline and control regarding floating-point semantics and
compiler optimizations to still implement the
Java-mandated results when optimization was enabled. The gcc family of
compilers, for example, has lacked such discipline.
oh, I see. Thanks for clarifying that. I was exactly wondering why fdlibm
optimization is off even for x86_x64 as it, AFAICS regarding gcc 5 only, does
not affect the precision, even if setting -O3 does not improve the performance
as much as on PPC64.
The fdlibm code relies on aliasing a two-element array of int with a double to
do bit-level reads and writes of floating-point values. As I understand it, the
C spec allows compilers to assume values
of different types don't overlap in memory. The compilation environment has to
be configured in such a way that the C compiler disables code generation and
optimization techniques that would run afoul
of these fdlibm coding practices.
On discussing with the Power toolchain folks we narrowed down the issue on PPC64
to the FMA. -fno-strict-aliasing has no effect and when used with an aggressive
optimization does not solve the issue on precision. Thus -ffp-contract=off is
the best options we have by now to optimize the fdlibm on PPC64.
Ah! I should have thought of this. I dealt with with fdlibm FMA issues
on ppc about 15 years ago. At the time -mno-fused-madd was the
solution. I don't think -ffp-contract=off existed back then.
Chris
Methods in the Math class, such as pow, are often intrinsified and use a
different algorithm so a straight performance comparison may not be as fair or
meaningful in those cases.
I agree. It's just that the issue on StrictMath methods was first noted due to
that huge gap (Math vs StrictMath) on PPC64, which is not prominent on x64.
Depending on how Math.{sin, cos} is implemented on PPC64, compiling the fdlibm
sin/cos with more aggressive optimizations should not be expected to close the
performance gap. In particular, if
Math.{sin, cos} is an intrinsic on PPC64 (I haven't checked the sources) that
used platform-specific feature (say fused multiply add instructions) then just
compiling fdlibm more aggressively wouldn't
necessarily make up that gap.
In our case (PPC64) it does close the gap. Non-optimized code will suffer a lot,
for instance, from load-hit-store issues. Contrary to what happens on PPC64, the
gap on x64 seems to be quite small as you said.
To allow cross-platform and cross-release reproducibility, StrictMath is
specified to use the particular fdlibm algorithms, which precludes using better
algorithms developed more recently. If we were
to start with a clean slate today, to get such reproducibility we would specify
correctly-rounded behavior of all those methods, but such an approach was much
less tractable technical 20+ years ago
without benefit of the research that was been done in the interim, such as the
work of Prof. Muller and associates:
https://lipforge.ens-lyon.fr/projects/crlibm/.
Accumulating the the results of the functions and comparisons the sums is not a
sufficiently robust way of checking to see if the optimized versions are indeed
equivalent to the non-optimized ones.
The specification of StrictMath requires a particular result for each set of
floating-point arguments and sums get round-away low-order bits that differ.
That's really good point, thanks for letting me know about that. I'll re-test my
change under that perspective.
Running the JDK math library regression tests and corresponding JCK tests is
recommended for work in this area.
Got it. By "the JDK math library regression tests" you mean exactly which test
suite? the jtreg tests?
Specifically, the regression tests under test/java/lang/Math and
test/java/lang/StrictMath in the jdk repository. There are some other math
library tests in the hotspot repo, but I don't know where
they are offhand.
A note on methodologies, when I've been writing test for my port I've tried to
include test cases that exercise all the branches point in the code. Due to the
large input space (~2^64 for a
single-argument method), random sampling alone is an inefficient way to try to
find differences in behavior.
For testing against JCK/TCK I'll need some help on that.
I believe the JCK/TCK does have additional testcases relevant here.
HTH; thanks,
-Joe
Thank you very much for the valuable comments.
I'll send a webrev accordingly for review.
I filed a bug: https://bugs.openjdk.java.net/browse/JDK-8170153
Best regards,
Gustavo
Re: PPC64: Poor StrictMath performance due to non-optimized compilation
On 11/17/16 1:33 PM, joe darcy wrote:
Hi Gustavo,
On 11/17/2016 10:31 AM, Gustavo Romero wrote:
Hi Joe,
Thanks a lot for your valuable comments.
On 17-11-2016 15:35, joe darcy wrote:
Currently, optimization for building fdlibm is disabled, except for
the
"solaris" OS target [1].
The reason for that is because historically the Solaris compilers
have had sufficient discipline and control regarding floating-point
semantics and compiler optimizations to still implement the
Java-mandated results when optimization was enabled. The gcc family
of compilers, for example, has lacked such discipline.
oh, I see. Thanks for clarifying that. I was exactly wondering why
fdlibm
optimization is off even for x86_x64 as it, AFAICS regarding gcc 5
only, does
not affect the precision, even if setting -O3 does not improve the
performance
as much as on PPC64.
The fdlibm code relies on aliasing a two-element array of int with a
double to do bit-level reads and writes of floating-point values. As I
understand it, the C spec allows compilers to assume values of
different types don't overlap in memory. The compilation environment
has to be configured in such a way that the C compiler disables code
generation and optimization techniques that would run afoul of these
fdlibm coding practices.
This is the strict aliasing issue right? It's a long standing problem
with fdlibm that kept getting worse as gcc got smarter. IIRC, compiling
with -fno-strict-aliasing fixes it, but it's been more than 12 years
since I last dealt with fdlibm and compiler aliasing issues.
Chris
As a consequence on PPC64 (Linux) StrictMath methods like, but not
limited to,
sin(), cos(), and tan() perform verify poor in comparison to the
same methods
in Math class [2]:
If you are doing your work against JDK 9, note that the pow, hypot,
and cbrt fdlibm methods required by StrictMath have been ported to
Java (JDK-8134780: Port fdlibm to Java). I have intentions to
port the remaining methods to Java, but it is unclear whether or not
this will occur for JDK 9.
Yes, I'm doing my work against 9. So is there any problem if I
proceed with my
change? I understand that there is no conflict as JDK-8134780
progresses and
replaces the StrictMath methods by their counterparts in Java.
Please, advice.
If I manage to finish the fdlibm C -> Java port in JDK 9, the changes
you are proposing would eventually be removed as unneeded since the C
code wouldn't be there to get compiled anymore.
Is it intended to downport JDK-8134780 to 8?
Such a backport would be technically possible, but we at Oracle don't
currently plan to do so.
Methods in the Math class, such as pow, are often intrinsified and
use a different algorithm so a straight performance comparison may
not be as fair or meaningful in those cases.
I agree. It's just that the issue on StrictMath methods was first
noted due to
that huge gap (Math vs StrictMath) on PPC64, which is not prominent
on x64.
Depending on how Math.{sin, cos} is implemented on PPC64, compiling
the fdlibm sin/cos with more aggressive optimizations should not be
expected to close the performance gap. In particular, if Math.{sin,
cos} is an intrinsic on PPC64 (I haven't checked the sources) that
used platform-specific feature (say fused multiply add instructions)
then just compiling fdlibm more aggressively wouldn't necessarily make
up that gap.
To allow cross-platform and cross-release reproducibility, StrictMath
is specified to use the particular fdlibm algorithms, which precludes
using better algorithms developed more recently. If we were to start
with a clean slate today, to get such reproducibility we would specify
correctly-rounded behavior of all those methods, but such an approach
was much less tractable technical 20+ years ago without benefit of the
research that was been done in the interim, such as the work of Prof.
Muller and associates: https://lipforge.ens-lyon.fr/projects/crlibm/.
Accumulating the the results of the functions and comparisons the
sums is not a sufficiently robust way of checking to see if the
optimized versions are indeed equivalent to the non-optimized ones.
The specification of StrictMath requires a particular result for
each set of floating-point arguments and sums get round-away
low-order bits that differ.
That's really good point, thanks for letting me know about that. I'll
re-test my
change under that perspective.
Running the JDK math library regression tests and corresponding JCK
tests is recommended for work in this area.
Got it. By "the JDK math library regression tests" you mean exactly
which test
suite? the jtreg tests?
Specifically, the regression tests under test/java/lang/Math and
test/java/lang/StrictMath in the jdk repository. There are some other
math library tests in the hotspot repo, but I don't know where they
are offhand.
A note on methodologies, when I've been writing test for my port I've
tried to include
Re: RFR: JDK-8155587: Cross compilation may cause compiler warnings for "build" compiler
Hi Erik, I can confirm the changes resolve the issues I was having. I looked at the changes, but don't really have the background to comment on them. thanks, Chris On 5/10/16 9:41 AM, Erik Joelsson wrote: Hello, When cross compiling, we compile a subset of the jdk for the build host to be able to run jmod and jlink. Configure sets up a toolchain for this through variables prefixed "BUILD_" (BUILD_CC etc). The new nifty macro we have for conditionally setting compiler flags depending on the version of the compiler is currently unaware of this. The result is that if the compiler version for the target and build toolchain differ, the macro will provide the wrong compiler flags. This patch makes the version check macro able to handle both the target and build toolchain. It also fixes the BUILD_CC_DISABLE_WARNING_PREFIX variable which is currently always empty. The result is that our cross compile configurations at Oracle no longer emits warnings from the build toolchain. Bug: https://bugs.openjdk.java.net/browse/JDK-8155587 Webrev: http://cr.openjdk.java.net/~erikj/8155587/webrev.top.01/ /Erik
Re: RFR: JDK-8152959: Build crashes in jdk9-hs-comp on Linux with gnumake 3.81
Hi Erik, I can confirm that this fixes the issue I was seeing. thanks, Chris On 3/29/16 6:14 AM, Erik Joelsson wrote: Hello, The VarHandles change introduced a new gensrc step that crashes when using gnumake 3.81. Newer versions of make seem to handle the construct in that file. I have found a way to express the makefile, which is a bit more correct, and doesn't crash. The relevant part of the change is the extra dollars inside the eval. An eval inside a define that is expected to be called using eval, requires double escaping of dollars for correct handling. Most of the time it works without though. In this case adding it seems to stop the crashing. The rest of the change is minor cleanups. I intend to push this to hs-comp since that's where the varhandles patch currently sits. Bug: https://bugs.openjdk.java.net/browse/JDK-8152959 Webrev: http://cr.openjdk.java.net/~erikj/8152959/webrev.jdk.01/ /Erik
Re: [RFR] (XS) 8144677: jprt.properties should allow creating a user specified testset with custom build flavors and build targets
Thanks David! Can I get a second reviewer please? thanks, Chris On 12/6/15 3:52 PM, David Holmes wrote: Hi Chris, On 5/12/2015 7:00 AM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8144677 http://cr.openjdk.java.net/~cjplummer/8144677/webrev.01/webrev/ Tested with JPRT with: • "-testset hotspot" • "-testset svc" • "-testset chris" from the example custom testset provided in the CR. • no testset specified Looks good! BTW, if anyone knows of an "include" mechanism for jprt.properties, Properties files do not have an include mechanism. See: http://docs.oracle.com/javase/8/docs/api/java/util/Properties.html#load-java.io.Reader- Thanks, David please let me know. Although that won't change the need for the above changes, it would make it possible to put custom testsets in a file rather than having to paste them in your ~/.jprt.properties file. thanks, Chris
Re: [RFR] (XS) 8144677: jprt.properties should allow creating a user specified testset with custom build flavors and build targets
Thanks Mikael! Chris On 12/7/15 3:22 PM, Mikael Vidstedt wrote: Thumbs up! Cheers, Mikael On 2015-12-07 14:27, Chris Plummer wrote: Thanks David! Can I get a second reviewer please? thanks, Chris On 12/6/15 3:52 PM, David Holmes wrote: Hi Chris, On 5/12/2015 7:00 AM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8144677 http://cr.openjdk.java.net/~cjplummer/8144677/webrev.01/webrev/ Tested with JPRT with: � "-testset hotspot" � "-testset svc" � "-testset chris" from the example custom testset provided in the CR. � no testset specified Looks good! BTW, if anyone knows of an "include" mechanism for jprt.properties, Properties files do not have an include mechanism. See: http://docs.oracle.com/javase/8/docs/api/java/util/Properties.html#load-java.io.Reader- Thanks, David please let me know. Although that won't change the need for the above changes, it would make it possible to put custom testsets in a file rather than having to paste them in your ~/.jprt.properties file. thanks, Chris
[RFR] (XS) 8144677: jprt.properties should allow creating a user specified testset with custom build flavors and build targets
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8144677 http://cr.openjdk.java.net/~cjplummer/8144677/webrev.01/webrev/ Tested with JPRT with: • "-testset hotspot" • "-testset svc" • "-testset chris" from the example custom testset provided in the CR. • no testset specified BTW, if anyone knows of an "include" mechanism for jprt.properties, please let me know. Although that won't change the need for the above changes, it would make it possible to put custom testsets in a file rather than having to paste them in your ~/.jprt.properties file. thanks, Chris
Re: [9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT
On 12/5/14 6:05 AM, Daniel D. Daugherty wrote: On 12/5/14 2:45 AM, David Holmes wrote: On 5/12/2014 7:22 PM, David Holmes wrote: On 5/12/2014 6:11 PM, Staffan Larsen wrote: Running with longer timeouts on fast machines makes the testing less responsive (if a test is on its way to timeout it takes longer for us to detect it). Ideally the timeout factor should be tuned according to the machine type we are running on. I�m not sure that is possible, though? We don't have that level of control unfortunately. Sorry that's not true. As this is our Makefile we could make the timeout value platform specific (though remember this is an open file). And if we have some means of defining a performance metric for a machine, we could even tune it for the current machine - but I'm not sure we'd want to go that path anyway. Perhaps a RFE. What Chris is proposing addresses a current problem and brings hotspot testing into line with what the JDK testing has been doing since 2009. :) I'm fairly certain that VM/SQE nightly tunes the JavaTest/JTREG timeout factor depending on the machine type or the machine name itself; I don't know the exact details. I suspect that this is possible in VM/SQE nightly/Aurora because there is a database of per-machine info. It might be possible to do something similar for JPRT if it tracks information per client machine... Ok. BTW, I was not suggesting a JPRT fix instead of my above fix in the near term. I still would like to push my fix and have the timeout for hotspot/test be on par with jdk/test. I still need one more reviewer. thanks, Chris Dan David David On 5 dec 2014, at 00:52, David Holmes david.hol...@oracle.com wrote: Hi Chris, Sorry I mis-directed you to send this one to build-dev, as it is a hotspot test/Makefile fix it should be reviewed by hotspot-dev now cc'd. Fix looks good to me. Thanks, David On 5/12/2014 6:37 AM, Chris Plummer wrote: Please review the following fix to address JPRT timeout issues when using -rtests to run hotspot JTReg tests on slow devices. The same logic has been in place for jdk/test/Makefile for a while now, so I just copied from there to hotspot/test/Makefile. https://bugs.openjdk.java.net/browse/JDK-8066508 http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/ thanks, Chris
Re: [9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT
Is the copyright rule/policy to leave the oldest date and update the most recent date to the current year? Chris On 12/5/14 1:48 PM, Daniel D. Daugherty wrote: http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/ test/Makefile No comments. Thumbs up. Please fix the Copyright year before you push. Dan On 12/4/14 4:52 PM, David Holmes wrote: Hi Chris, Sorry I mis-directed you to send this one to build-dev, as it is a hotspot test/Makefile fix it should be reviewed by hotspot-dev now cc'd. Fix looks good to me. Thanks, David On 5/12/2014 6:37 AM, Chris Plummer wrote: Please review the following fix to address JPRT timeout issues when using -rtests to run hotspot JTReg tests on slow devices. The same logic has been in place for jdk/test/Makefile for a while now, so I just copied from there to hotspot/test/Makefile. https://bugs.openjdk.java.net/browse/JDK-8066508 http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/ thanks, Chris
[9] RFR (XS) 8066507: JPRT is not capable of running jtreg tests located jdk/test
Please review the following fix to allow running jdk jtreg tests using JPRT -rtests. This is the same fix that his been in place in hotspot/test/Makefile for a while now, so I just copied from there to jdk/test/Makefile. https://bugs.openjdk.java.net/browse/JDK-8066507 http://cr.openjdk.java.net/~cjplummer/8066507/webrev.00/ thanks, Chris
[9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT
Please review the following fix to address JPRT timeout issues when using -rtests to run hotspot JTReg tests on slow devices. The same logic has been in place for jdk/test/Makefile for a while now, so I just copied from there to hotspot/test/Makefile. https://bugs.openjdk.java.net/browse/JDK-8066508 http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/ thanks, Chris
