Re: RFR: 8292758: put support for UNSIGNED5 format into its own header file [v2]

On Fri, 2 Sep 2022 00:04:17 GMT, John R Rose wrote: >> Refactor code from inside of CompressedStream into its own unit. >> >> This code is likely to be used in future refactorings, such as JDK-8292818 >> (replace 96-bit representation for field metadata with variable-sized >> streams). >> >>

Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v6]

On Thu, 18 Aug 2022 09:32:54 GMT, Axel Boldt-Christmas wrote: >> src/hotspot/share/code/compiledMethod.cpp line 128: >> >>> 126: if (old_status < new_status) { >>> 127: if (old_status == not_enqueued) { >>> 128: >>>

Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v6]

On Thu, 18 Aug 2022 08:18:27 GMT, Axel Boldt-Christmas wrote: >> The proposal is to encapsulate the nmethod mark for deoptimization logic in >> one place and only allow access to the `mark_for_deoptimization` from a >> closure object: >> ```C++ >> class DeoptimizationMarkerClosure : StackObj

Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v2]

On Mon, 1 Aug 2022 04:58:49 GMT, Axel Boldt-Christmas wrote: >> The proposal is to encapsulate the nmethod mark for deoptimization logic in >> one place and only allow access to the `mark_for_deoptimization` from a >> closure object: >> ```C++ >> class DeoptimizationMarkerClosure : StackObj {

Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v2]

On Wed, 3 Aug 2022 20:42:59 GMT, Erik Österlund wrote: >> Axel Boldt-Christmas has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Add assertions >> - Fix marked logic >> - Erik refactorings > > Also, in

Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v2]

On Mon, 1 Aug 2022 04:58:49 GMT, Axel Boldt-Christmas wrote: >> The proposal is to encapsulate the nmethod mark for deoptimization logic in >> one place and only allow access to the `mark_for_deoptimization` from a >> closure object: >> ```C++ >> class DeoptimizationMarkerClosure : StackObj {

Re: RFR: 8290373: Enable lossy conversion warnings on Windows

On Fri, 15 Jul 2022 13:25:57 GMT, Jorn Vernee wrote: > This patch enables lossy conversion warnings (C4244 [1]) for hotspot on > Windows/MSVC. Instead of fixing all warnings that were produced from this, > I've instead locally disabled the warning in the files that produced > warnings. This

Re: State of x86_32 port of JEP 425: Virtual Threads (Preview)

On 6/15/22 9:54 AM, Aleksey Shipilev wrote: (I *suspect* that somewhere we stash the "current" thread onto stack, and then remount to another thread, which makes "current" thread obsolete. Assuming current thread does not change is quite pervasive, and saving "current thread" on

Re: RFR: 8283849: AsyncGetCallTrace may crash JVM on guarantee [v6]

On Fri, 6 May 2022 09:40:37 GMT, Jaroslav Bachorik wrote: >> A gist of the fix is to allow relaxed special handling of code blob lookup >> when done for ASGCT. >> >> Currently, a guarantee will fail when we happen to hit a zombie method which >> is still on stack. While this would indicate

Re: RFR: 8283849: AsyncGetCallTrace may crash JVM on guarantee [v6]

On Fri, 6 May 2022 09:40:37 GMT, Jaroslav Bachorik wrote: >> A gist of the fix is to allow relaxed special handling of code blob lookup >> when done for ASGCT. >> >> Currently, a guarantee will fail when we happen to hit a zombie method which >> is still on stack. While this would indicate

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in

Re: [jdk18] RFR: 8281713: [BACKOUT] AArch64: Implement string_compare intrinsic in SVE

On Mon, 14 Feb 2022 10:30:09 GMT, Tobias Hartmann wrote: > We observed several failures on macosx aarch64 after integration of > [JDK-8275448](https://bugs.openjdk.java.net/browse/JDK-8275448). I could > reliably reproduce > [JDK-8281512](https://bugs.openjdk.java.net/browse/JDK-8281512) with

Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination

On Mon, 13 Dec 2021 09:39:55 GMT, Сергей Цыпанов wrote: > Originally this was spotted by by Amir Hadadi in > https://stackoverflow.com/questions/70272651/missing-bounds-checking-elimination-in-string-constructor > > It looks like in the following code in `String(byte[], int, int, Charset)` >

Re: [jdk18] RFR: 8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation

On Wed, 15 Dec 2021 05:59:45 GMT, Vladimir Kozlov wrote: > A proper fix for this is to use the catchException combination. However, that > introduces significant cold startup performance regression. JDK-8278447 > tracks the work to address the performance regression using catchException > and

Re: RFR: 8218885: Restore pop_frame and force_early_return functionality for Graal

On Wed, 22 Sep 2021 05:40:40 GMT, Tom Rodriguez wrote: > This logic no longer seems to be necessary since the adjustCompilationLevel > callback has been removed. Marked as reviewed by dlong (Reviewer). Make sure to test with -Xcomp. - PR:

Re: RFR: 8273459: Update code segment alignment to 64 bytes

On Thu, 16 Sep 2021 13:52:24 GMT, Scott Gibbons wrote: > Change the default code entry alignment to 64 bytes from 32 bytes. This > allows for maintaining proper 64-byte alignment of data within a code > segment, which is required by several AVX-512 instructions. > > I ran into this while

Re: RFR: 8270083: -Wnonnull errors happen with GCC 11.1.1

On Fri, 9 Jul 2021 00:28:38 GMT, Yasumasa Suenaga wrote: >> I attempted to build OpenJDK on Fedora 34 with gcc-11.1.1-3.fc34.x86_64, but >> I saw following errors: >> >> >> In file included from >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/runtime/frame.inline.hpp:42, >>

Re: RFR: 8270083: -Wnonnull errors happen with GCC 11.1.1

On Thu, 8 Jul 2021 09:42:43 GMT, Yasumasa Suenaga wrote: > I attempted to build OpenJDK on Fedora 34 with gcc-11.1.1-3.fc34.x86_64, but > I saw following errors: > > > In file included from > /home/ysuenaga/github-forked/jdk/src/hotspot/share/runtime/frame.inline.hpp:42, >

Re: RFR: 8270083: -Wnonnull errors happen with GCC 11.1.1

On Thu, 8 Jul 2021 09:42:43 GMT, Yasumasa Suenaga wrote: > I attempted to build OpenJDK on Fedora 34 with gcc-11.1.1-3.fc34.x86_64, but > I saw following errors: > > > In file included from > /home/ysuenaga/github-forked/jdk/src/hotspot/share/runtime/frame.inline.hpp:42, >

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

On Fri, 9 Apr 2021 16:54:51 GMT, Ioi Lam wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > LGTM. Just a small nit. @iklam I thought the fingerprint code was also

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

On Fri, 9 Apr 2021 16:54:51 GMT, Ioi Lam wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > LGTM. Just a small nit. @iklam I thought the fingerprint code was also

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

On Fri, 9 Apr 2021 16:54:51 GMT, Ioi Lam wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > LGTM. Just a small nit. @iklam I thought the fingerprint code was also

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v7]

On Tue, 17 Nov 2020 08:33:20 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

On Thu, 5 Nov 2020 23:58:21 GMT, Dean Long wrote: >> Roland Westrelin has updated the pull request with a new target base due to >> a merge or a rebase. The pull request now contains ten commits: >> >> - Jorn's comments >> - Update headers and add intri

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

On Thu, 5 Nov 2020 15:47:31 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long

Re: RFR: 8255452: Doing GC during JVMTI MethodExit event posting breaks return oop

On Thu, 29 Oct 2020 12:44:58 GMT, Erik Österlund wrote: > The imasm::remove_activation() call does not deal with safepoints very well. > However, when the MethodExit JVMTI event is being called, we call into the > runtime in the middle of remove_activation(). If the value being returned is >

Re: RFR: 8255452: Doing GC during JVMTI MethodExit event posting breaks return oop

On Fri, 30 Oct 2020 06:56:13 GMT, Richard Reingruber wrote: >> Changes requested by coleenp (Reviewer). > > Hi Erik, > > is it possible for GC to mistake a primitive value for a reference when > posting the exit event? > > My understanding is: we are at a random bci of a method that is forced

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

On 5/28/20 10:54, Dean Long wrote: Sure, you could just have cache_jvmti_state() return a boolean to bail out immediately for is_old. dl On 5/28/20 7:23 AM, serguei.spit...@oracle.com wrote: Hi Dean, Thank you for looking at this! Okay. Let me check what cab be done in this dir

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

. The compilation has to bail out if it is discovered to be true. Thanks, Serguei On 5/28/20 00:59, Dean Long wrote: This seems OK as long as the memory barriers in the thread state transitions prevent the C++ compiler from doing something like reading is_old before reading redefinition_count.  I

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

This seems OK as long as the memory barriers in the thread state transitions prevent the C++ compiler from doing something like reading is_old before reading redefinition_count.  I would feel better if both JVMCI and C1/C2 cached is_old and redefinition_count at the same time (making sure to

Re: (S) 8237750: Load libzip.so only if necessary

/webrev-01/   Tested locally, passed jtreg/runtime. Thanks Yumin On 5/1/20 4:24 PM, Dean Long wrote: OK, I didn't realize compute_fingerprint is using ZIP_CRC32. dl On 5/1/20 2:42 PM, Yumin Qi wrote: Hi, Dean   Thanks for the review. I will try MutextLocker, for AOT, I think currently

Re: (S) 8237750: Load libzip.so only if necessary

n Qi wrote: Dean,   I have updated to use MutexLocker instead at same link: http://cr.openjdk.java.net/~minqi/8237750/webrev-01/   Tested locally, passed jtreg/runtime. Thanks Yumin On 5/1/20 4:24 PM, Dean Long wrote: OK, I didn't realize compute_fingerprint is using ZIP_CRC32. dl On 5/1/

Re: (S) 8237750: Load libzip.so only if necessary

.c:333 #27 0x7790041d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 This is not with CDS. Thanks Yumin On 5/1/20 2:11 PM, Dean Long wrote: It looks like Zip_lock could be a MutexLocker instead of a MonitorLocker. dl On 5/1/20 10:24 AM, Yumin Qi wrote: Hi, please review: bug 8237

Re: (S) 8237750: Load libzip.so only if necessary

It looks like Zip_lock could be a MutexLocker instead of a MonitorLocker. dl On 5/1/20 10:24 AM, Yumin Qi wrote: Hi, please review: bug 8237750: https://bugs.openjdk.java.net/browse/JDK-8237750 webrev: http://cr.openjdk.java.net/~minqi/8237750/webrev-01/ Summary:   zip library is loaded

Re: (S) 8237750: Load libzip.so only if necessary

Does UseAOT really need libzip? dl On 5/1/20 10:24 AM, Yumin Qi wrote: Hi, please review: bug 8237750: https://bugs.openjdk.java.net/browse/JDK-8237750 webrev: http://cr.openjdk.java.net/~minqi/8237750/webrev-01/ Summary:   zip library is loaded unconditionally at start up but it is only

Re: RFR (M) Close alignment gaps in InstanceKlass

OK, thanks, looks good! dl On 4/22/20 7:32 PM, coleen.phillim...@oracle.com wrote: On 4/22/20 9:00 PM, Dean Long wrote: It looks like calling the JVMCI getSourceFileName() on an array would have accessed random memory because it was expecting an InstanceKlass.  Instead of returning null we

Re: RFR (M) Close alignment gaps in InstanceKlass

It looks like calling the JVMCI getSourceFileName() on an array would have accessed random memory because it was expecting an InstanceKlass.  Instead of returning null we might want to throw an exception like in HotSpotResolvedPrimitiveType. dl On 4/22/20 5:39 PM, Dean Long wrote: Can you

Re: RFR (M) Close alignment gaps in InstanceKlass

the other tests in that file. http://cr.openjdk.java.net/~coleenp/2020/8238048.02.incr/webrev/index.html Thanks, Coleen On 4/21/20 9:51 PM, Dean Long wrote: Hi Coleen.  The JVMCI changes look OK.  It looks like there is a Graal unittest that covers getSourceFileName, but those tests don't

Re: RFR (M) Close alignment gaps in InstanceKlass

Hi Coleen.  The JVMCI changes look OK.  It looks like there is a Graal unittest that covers getSourceFileName, but those tests don't always get run.  If it's not too much trouble, could you look into enabling getSourceFileName() testing in

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

I looked at the AOT, C2, and JVMCI changes and I didn't find any issues. dl On 3/26/20 4:57 PM, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area. Small changes are made in javac, VM compiler

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

I looked at the AOT, C2, and JVMCI changes and I didn't find any issues. dl On 3/26/20 4:57 PM, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area. Small changes are made in javac, VM compiler

Re: RFR(S) 8238585: Use handshake for JvmtiEventControllerPrivate::enter_interp_only_mode() and don't make compiled methods on stack not_entrant

You might want to have some runtime/GC folks look at the handshake changes. dl On 2/6/20 4:39 AM, Reingruber, Richard wrote: Hi, could I please get reviews for this small enhancement: Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8238585/webrev.0/ Bug:

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

This might be a dumb question, but how is PopFrame used in practice?  Re-invoking the method, especially with modified argument values seems dangerous. dl

Re: RFR(XXS) 8231902: Build of jdk.internal.vm.compiler.management/module-info.java.extra failed

Thanks Magnus and Erik! dl On 10/7/19 8:20 AM, Erik Joelsson wrote: Looks good! Thanks! /Erik On 2019-10-04 15:09, dean.l...@oracle.com wrote: Here's an alternative version with $(NAWK) usage suggested by Erik Joelsson: http://cr.openjdk.java.net/~dlong/8231902/webrev.2/ dl On 10/4/19

Re: RFR(XXS) 8231902: Build of jdk.internal.vm.compiler.management/module-info.java.extra failed

Thanks Vladimir. dl On 10/4/19 1:03 PM, Vladimir Kozlov wrote: Good. Thanks Vladimir On Oct 4, 2019, at 12:29 PM, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8231902 http://cr.openjdk.java.net/~dlong/8231902/webrev/ A recent upstream Graal change causes the

Re: RFR(XXS) 8231902: Build of jdk.internal.vm.compiler.management/module-info.java.extra failed

Here's an alternative version with $(NAWK) usage suggested by Erik Joelsson: http://cr.openjdk.java.net/~dlong/8231902/webrev.2/ dl On 10/4/19 12:29 PM, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8231902 http://cr.openjdk.java.net/~dlong/8231902/webrev/ A recent

RFR(XXS) 8231902: Build of jdk.internal.vm.compiler.management/module-info.java.extra failed

https://bugs.openjdk.java.net/browse/JDK-8231902 http://cr.openjdk.java.net/~dlong/8231902/webrev/ A recent upstream Graal change causes the META-INF/providers directory to contain only a single file, causing the $(GREP) command to not prepend the filename to the output.  The simple fix is to

Re: RFR 8226690: SIGSEGV in MetadataOnStackClosure::do_metadata

Looks good. dl On 9/25/19 6:29 PM, coleen.phillim...@oracle.com wrote: On 9/25/19 9:21 PM, coleen.phillim...@oracle.com wrote: I see.  I dumped the redefinition count in the replay data because I saw the other fields were dumped there.  Would they also not affect the generated code? I

Re: RFR 8226690: SIGSEGV in MetadataOnStackClosure::do_metadata

On 9/25/19 6:21 PM, coleen.phillim...@oracle.com wrote: I see.  I dumped the redefinition count in the replay data because I saw the other fields were dumped there.  Would they also not affect the generated code? I know some like _jvmti_can_access_local_variables can affect the generated

Re: RFR 8226690: SIGSEGV in MetadataOnStackClosure::do_metadata

Saving and restoring redefinition_count for compiler replay doesn't make sense to me.  It won't affect the generated code, and we probably shouldn't even be installing/registering replay nmethods. I would remove the ciEnv::dump_replay_data_unsafe() and process_JvmtiExport() changes. dl On

Re: 8195600: [Graal] jdi tests timeouts with Graal because debuggee vm is not resumed

I don't have a strong opinion either way.  It's too bad we don't have resource management features that would allow setting a memory limit on the test or app while allowing the rest of the JVM to use memory unrestricted.  That might solve a lot of these OOM problems. Even after we move to

Re: RFR: 8195600: [Graal] jdi tests timeouts with Graal because debuggee vm is not resumed

Good question  When we have libgraal, there will still be an option (at least for debugging) to turn it off and use Graal the same way we do now, so it seems like the @requires would need to take that into account once we have libgraal.  Maybe we will need a new "vm.libgraal.enabled" or make

Re: RFR: 8195600: [Graal] jdi tests timeouts with Graal because debuggee vm is not resumed

This is the kind of failure that is expected to go away with libgraal.  You can add the tests to the Graal-specific problem list (see JDK-8196611) and they should be re-enabled with libgraal (see JDK-JDK-8207267). dl On 8/8/19 10:21 AM, Chris Plummer wrote: Hi Daniil, My only objection is

Re: RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

The ** syntax isn't working for me in Mercurial 2.9, but the following: syntax: glob JTreport/* JTwork/* seems to work in both 2.9 and 4.6.1.  This also seems to work: syntax: glob JTreport JTwork but it matches files and not just directories. dl On 7/10/19 4:48 AM, Jaikiran Pai wrote:

Re: RFR (S) 8225437: JvmtiExport::gc_epilogue is unnecessary

I don't see it removed from the .hpp files. dl On 6/26/19 8:02 AM, coleen.phillim...@oracle.com wrote: Summary: Remove jvmtiExport::gc_epilogue after full GCs See bug for more information. open webrev at http://cr.openjdk.java.net/~coleenp/2019/8225437.01/webrev bug link

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

Isn't that going to break inline asm? dl On 5/20/19 3:40 PM, Erik Joelsson wrote: That sounds good to me. I missed that distinction before, but pure c99 does sound better. /Erik On 2019-05-20 15:31, David Holmes wrote: Hi Volker, Thanks for the xlc update - I will incorporate that. It

Re: RFR: 8222821: com/sun/jdi/ExceptionEvents.java failed

Looks good. dl On 4/25/19 6:33 PM, Daniil Titov wrote: Please review the change that fixes an intermittent failure of the test when running with Graal on. The test creates exception requests for different kinds of exceptions and errors, starts the debuggee that throws an exception, and

Re: RFR: 8221723: Avoid storing zero to String.hash

OK, I guess there's no ideal solution.  Adding a separate "not-computed" boolean adds space, and using a different sentinel value for "not-computed" would probably be slower on CPUs that have a fast compare-and-branch-against-zero instruction. dl On 4/1/19 12:55 PM, Martin Buchholz wrote:

Re: RFR: 8221723: Avoid storing zero to String.hash

Wouldn't it be better to write a non-0 value when the computed hash code is 0, so we don't have to recompute it?  Is there some advantage to writing 0 instead of any other value, such as 1? dl On 4/1/19 4:57 AM, Claes Redestad wrote: Hi, when a String has a calculated hash code value of 0,

Re: RFR: JDK-8218166: [Graal] com/sun/jdi/SimulResumerTest.java failure

I don't know how to interpret "ignore checks if thread was collected", so it doesn't add much value for me. How about something like "ignore ownedMonitors() failure"? dl On 3/13/19 8:54 AM, Gary Adams wrote: One last set of diffs ...   - added comments on the ignored exceptions   - commented

Re: RFR: 8220244: vmTestbase/nsk/jvmti/scenarios/sampling/SP06/sp06t003 hasn't been un-problemlisted

Looks good and trivial. dl On 3/12/19 4:46 PM, Daniil Titov wrote: Please review a trivial change that removes test vmTestbase/nsk/jvmti/scenarios/sampling/SP06/sp06t003, recently fixed in 8218167, from ProblemList-graal.txt Webrev: http://cr.openjdk.java.net/~dtitov/8220244/webrev.01/ Bug:

Re: RFR: JDK-8218166: [Graal] com/sun/jdi/SimulResumerTest.java failure

Looks good to me. dl On 3/12/19 11:07 AM, Gary Adams wrote: Still need 2 more reviewers ... On 3/7/19, 9:45 AM, Daniel D. Daugherty wrote: Gary, Since the 'graal' label was recently removed, I also removed "[Graal]" from the bug synopsis. Please make sure you update your commit mesg. On

Re: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed

This is consistent with how other system threads are handled, so it looks good to me. dl On 3/11/19 4:32 PM, Daniil Titov wrote: Hi Dean, JC and Daniel, Thank you for reviewing this change. Based on the overall output it seems as the common consensus is that the broader discussions is

Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed

This part seems to change the value for the include_jni_attaching_threads arg from true to false: - ThreadsListEnumerator tle(Thread::current(), true); + ThreadsListEnumerator tle(Thread::current(), true, false, JvmtiExport::should_show_compiler_threads()); dl

Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed

There are other places where is_hidden_from_external_view() is used.  Should is_hidden_from_external_view() also check the new capability?  If so, then you can simplify your changes.  I'm not sure the new capability is the best choice, however.  There is still no way to control whether

Re: RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails

Looks good, but what about sp06t003?  Doesn't it have the same problem?  Are there any other tests using similar logic? dl On 3/1/19 8:33 PM, Daniil Titov wrote: Please review the change that fix intermittent failure for test nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with

Re: RFR (M) 8139551: Scalability problem with redefinition - multiple code cache walks

Could you move this code: // Mark dependent AOT nmethods, which are only found via the class redefined. AOTLoader::mark_evol_dependent_methods(ik); into this function: CodeCache::mark_for_evol_deoptimization(ik) The rest looks good. dl On 2/4/19 7:08 AM,

Re: RFR (12) JDK-8218025: disable pop_frame and force_early_return caps for Graal

On 1/30/19 8:59 PM, serguei.spit...@oracle.com wrote: So, the fix needs to be more like this: + // Workaround for 8195635: + // disable pop_frame and force_early_return capabilities with Graal + #if INCLUDE_JVMCI + if (!(EnableJVMCI && UseJVMCICompiler)) { jc.can_pop_frame = 1;

Re: 12 RFR(M) 8195635: [Graal] nsk/jvmti/unit/ForceEarlyReturn/earlyretbase crashes with assertion "compilation level out of bounds"

I'm withdrawing this RFR for 12. dl On 1/28/19 5:13 PM, dean.l...@oracle.com wrote: http://cr.openjdk.java.net/~dlong/8195635/webrev.5/ https://bugs.openjdk.java.net/browse/JDK-8195635 Please see the bug report for all the gory details.  Here's the short version: If we allow any safepoint

12 RFR(M) 8195635: [Graal] nsk/jvmti/unit/ForceEarlyReturn/earlyretbase crashes with assertion "compilation level out of bounds"

http://cr.openjdk.java.net/~dlong/8195635/webrev.5/ https://bugs.openjdk.java.net/browse/JDK-8195635 Please see the bug report for all the gory details.  Here's the short version: If we allow any safepoint to be a suspend point, we run into trouble with PopFrame and ForceEarlyReturn, which

Re: RFR: 8217618: JVM TI SuspendThread doesn't suspend the current thread before returning

On 1/26/19 7:43 PM, Daniel D. Daugherty wrote: However, java_suspend_self() is careful and only self-suspends if is_external_suspend() is still true (and it makes that check while it owns the SR_lock). The code in java_suspend_self() has to be careful in both directions. You don't want it to

Re: RFR: 8217618: JVM TI SuspendThread doesn't suspend the current thread before returning

I think there is going to be a race with resume wherever you put the self-suspend, because we do it without holding SR_lock.  So you should be able to move the self check and self suspend to before the SR_lock.  This would just be a performance optimization. dl On 1/24/19 6:46 PM, David

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

Claes, can I list you as a reviewer? dl On 12/16/18 9:52 PM, Claes Redestad wrote: Fair enough, I wasn't aware EA was looking beyond the inlined code like this (which means it can't be "dead" or the JIT might see through the trick at some point). I've skimmed the existing usages in the JDK and

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

David, can I list you as a reviewer? dl On 12/16/18 8:47 PM, dean.l...@oracle.com wrote: On 12/16/18 7:39 PM, dean.l...@oracle.com wrote: On 12/16/18 7:03 PM, David Holmes wrote: On 17/12/2018 12:49 pm, dean.l...@oracle.com wrote: On 12/16/18 4:06 PM, David Holmes wrote: On 15/12/2018

Re: 12 RFR(XS) 8214329: SwingMark SubMenus 9% regression in 12-b19 on Linux client

On 12/18/18 7:12 AM, Sean Mullan wrote: Looks good, although I think someone from the HotSpot Group should also review it. Thanks Sean.  Tobias reviewed it. On the bug, can you add the details below into the Description? Sure. Also, since you have no regression test, you will need a

Re: 12 RFR(XS) 8214329: SwingMark SubMenus 9% regression in 12-b19 on Linux client

Thanks Tobias. dl On 12/18/18 4:08 AM, Tobias Hartmann wrote: Hi Dean, this looks good to me. Best regards, Tobias On 17.12.18 23:49, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8214329 http://cr.openjdk.java.net/~dlong/8214329/webrev/ In "8212605: Pure-Java

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

Thanks Mandy. dl On 12/17/18 2:55 PM, Mandy Chung wrote: This looks okay to me. Mandy On 12/14/18 4:59 PM, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8214583 http://cr.openjdk.java.net/~dlong/8214583/webrev This change includes two new regression test that

12 RFR(XS) 8214329: SwingMark SubMenus 9% regression in 12-b19 on Linux client

https://bugs.openjdk.java.net/browse/JDK-8214329 http://cr.openjdk.java.net/~dlong/8214329/webrev/ In "8212605: Pure-Java implementation of AccessController.doPrivileged", the stackwalk in JVM_GetStackAccessControlContext was changed from using a vframeStream to using a javaVFrame, so that it

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

On 12/16/18 9:52 PM, Claes Redestad wrote: Fair enough, I wasn't aware EA was looking beyond the inlined code like this (which means it can't be "dead" or the JIT might see through the trick at some point). I've skimmed the existing usages in the JDK and can't find anything that seems to be

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

On 12/16/18 9:52 PM, Claes Redestad wrote: Either way, it might be nice to have a more explicit facility for this in a future release, as David suggested. Say an @Escaping local variable/parameter annotation. I agree.  I suggested @Escapes or @StackWalkable in JDK-8214585. @Escaping or

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

On 12/16/18 7:39 PM, dean.l...@oracle.com wrote: On 12/16/18 7:03 PM, David Holmes wrote: On 17/12/2018 12:49 pm, dean.l...@oracle.com wrote: On 12/16/18 4:06 PM, David Holmes wrote: On 15/12/2018 10:59 am, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8214583

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

Unfortunately, I don't think @DontInline on an empty method is sufficient here.  If other code is relying on @DontInline for the same purpose then we might need to reexamine that code.  My understanding from discussing with other compiler engineers is that using a native method is the safest

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

On 12/16/18 7:03 PM, David Holmes wrote: On 17/12/2018 12:49 pm, dean.l...@oracle.com wrote: On 12/16/18 4:06 PM, David Holmes wrote: On 15/12/2018 10:59 am, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8214583 http://cr.openjdk.java.net/~dlong/8214583/webrev This

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

If we are adding JDK_EXPORT, does it make sense to add JDK_CALL as well? dl On 12/16/18 4:57 PM, David Holmes wrote: Hi Magnus, Thanks for explaining how addition of JNIEXPORT may have started this problem. One follow up: This will also need a CSR request due to the change in linking

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

If we are adding JDK_EXPORT, does it make sense to add JDK_CALL as well? dl On 12/16/18 4:57 PM, David Holmes wrote: Hi Magnus, Thanks for explaining how addition of JNIEXPORT may have started this problem. One follow up: This will also need a CSR request due to the change in linking

Re: 12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

On 12/16/18 4:06 PM, David Holmes wrote: On 15/12/2018 10:59 am, dean.l...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8214583 http://cr.openjdk.java.net/~dlong/8214583/webrev This change includes two new regression test that demonstrate the problem, and a fix that allows the

12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

https://bugs.openjdk.java.net/browse/JDK-8214583 http://cr.openjdk.java.net/~dlong/8214583/webrev This change includes two new regression test that demonstrate the problem, and a fix that allows the tests to pass. The problem happens when the JIT compiler's escape analysis eliminates the

Re: RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

On 11/30/18 7:46 PM, JC Beyler wrote: Questions because I'm not familiar with JVMCI consequences so not really comments on the webrev but so that I know:   - Is it normaly that you can suspend when you are in a JVMCI frame? Yes, because it's just Java code, and we allow all Java code to be

Re: RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode

OK your fix looks good. dl On 11/28/18 10:57 PM, Igor Veresov wrote: It would work right now. But I don’t want us fixing it again when somebody implements effectively final optimization in Graal. igor On Nov 28, 2018, at 10:02 PM, dean.l...@oracle.com wrote:

Re: RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode

I like it better.  But do you really need to use JNI to reset the values?  If you had static int POISON = 0x1234;  // not final class TaggedClass {   static int field1 = 0xC0DE01 + POISON; in HeapFilter.java, is the compiler smart enough to treat the value as constant until it changes?

Re: RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode

It sounds like the test could also fail with C2 if the fields are in a virtual object that was eliminated.  I'm OK with your fix, but I would feel a little better if we only relaxed the check for Graal. I guess you'd need to use the whitebox api for that. dl On 11/28/18 2:28 PM, Igor Veresov

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

I guess even with -Xcomp, there are still compiles triggered by profiled code, otherwise I don't see why changing policy()->event() helps. Another problem I see with this change is that it will make the real problem harder to reproduce. dl On 11/28/18 12:36 PM, Alex Menkov wrote: Hi guys,

Re: RFR(XS) 8193577: nsk/jvmti/IterateThroughHeap/filter-tagged fails with Graal in Xcomp mode

I'm guessing Graal creates Constant boxes of the individual fields and not of the test objects?  If so, can't we fix the matching so that it checks that all fields of an object match?  I guess that would mean moving the "expected" and "found" counts up from fields[] to objects_info[]. dl On

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

OK, reconsidering solution 1, I have a couple more questions: The bug report says the problem is reproducible with -Xcomp, but I don't see the policy()->event() path taken with -Xcomp.  Instead, -Xcomp uses CompilationPolicy::compile_if_required, so I don't see how Solution 1 fixes the

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

Let me list the proposed solutions: 1. short-circuit compiles in CompilationPolicy::event in is_interp_only_mode 2. detect is_interp_only_mode in resolve stubs and force a c2i call 3. don't offer a JVMTI suspend point in resolve stubs (or JavaCallWrapper) Solution 1 penalizes other threads, and

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

How does this solve the problem of HotSpotJVMCIRuntime.adjustCompilationLevel being called? I don't think this fix is the right approach.  Doesn't is_interp_only_mode() only apply to the current thread?  I don't think it's safe to assume no compiles will happen in other threads, or that a

Re: JNI - question about jmethodid values.

Doesn't the spec say it's the caller's responsibility to keep the class from being unloaded? " A field or method ID does not prevent the VM from unloading the class from which the ID has been derived. After the class is unloaded, the method or field ID becomes invalid. The native code,

<    1   2   3   4   >