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

2022-05-06 Thread Dean Long
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]

2022-05-06 Thread Dean Long
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]

2022-04-27 Thread Dean Long
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]

2022-04-27 Thread Dean Long
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: 8218885: Restore pop_frame and force_early_return functionality for Graal

2021-10-01 Thread Dean Long
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: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

2021-04-09 Thread Dean Long
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: 8255452: Doing GC during JVMTI MethodExit event posting breaks return oop

2020-10-30 Thread Dean Long
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

2020-10-30 Thread Dean Long
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

2020-06-01 Thread Dean Long
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

2020-05-28 Thread Dean Long
. 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

2020-05-28 Thread Dean Long
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

2020-05-04 Thread Dean Long
/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

2020-05-02 Thread Dean Long
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

2020-05-01 Thread Dean Long
.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

2020-05-01 Thread Dean Long
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

2020-05-01 Thread Dean Long
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

2020-04-23 Thread Dean Long
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

2020-04-22 Thread Dean Long
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

2020-04-22 Thread Dean Long
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

2020-04-21 Thread Dean Long
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

2020-03-27 Thread Dean Long
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

2020-02-11 Thread Dean Long
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"

2019-12-06 Thread Dean Long
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 8226690: SIGSEGV in MetadataOnStackClosure::do_metadata

2019-09-25 Thread dean . long
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

2019-09-25 Thread dean . long
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

2019-09-25 Thread dean . long
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

2019-08-27 Thread dean . long
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

2019-08-09 Thread dean . long
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

2019-08-08 Thread dean . long
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 (S) 8225437: JvmtiExport::gc_epilogue is unnecessary

2019-06-26 Thread dean . long
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: 8222821: com/sun/jdi/ExceptionEvents.java failed

2019-04-25 Thread dean . long
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: JDK-8218166: [Graal] com/sun/jdi/SimulResumerTest.java failure

2019-03-13 Thread dean . long
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

2019-03-12 Thread dean . long
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

2019-03-12 Thread dean . long
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

2019-03-11 Thread dean . long
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

2019-03-07 Thread dean . long
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

2019-03-07 Thread dean . long
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

2019-03-01 Thread dean . long
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

2019-02-04 Thread dean . long
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

2019-01-30 Thread dean . long
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"

2019-01-29 Thread dean . long
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"

2019-01-28 Thread dean . long
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

2019-01-27 Thread dean . long
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

2019-01-26 Thread dean . long
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: RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

2018-12-03 Thread dean . long
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

2018-11-29 Thread dean . long
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

2018-11-28 Thread dean . long
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

2018-11-28 Thread dean . long
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

2018-11-28 Thread dean . long
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

2018-11-28 Thread dean . long
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

2018-11-28 Thread dean . long
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

2018-11-27 Thread dean . long
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

2018-11-26 Thread dean . long
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.

2018-11-26 Thread dean . long
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,

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

2018-11-09 Thread dean . long
I see, eventually it should stop waiting and timeout.  That seems fine. dl On 11/9/18 12:26 PM, Daniil Titov wrote: Hi Dean, The blocking issue for suspended JVMCI compiler thread in case of Graal and -XComp was recently solved in JDK-8195627 " Graal]

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

2018-11-09 Thread dean . long
With OSR, a compile can be initiated in a for loop, so I'd say this is still not safe if the compile can be blocking, which happens if:     bool is_blocking = !directive->BackgroundCompilationOption || CompileTheWorld || ReplayCompiles; So that probably means the test cannot be run with

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

2018-11-03 Thread dean . long
I don't think suspending compiler threads is safe, especially if compiles are blocking/synchronous. dl On 11/2/18 4:29 PM, Daniil Titov wrote: Please review the change that fixes several tests failing with com.sun.jdi.ObjectCollectedException when running with Graal. There main problem here

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-24 Thread dean . long
On 10/23/18 11:04 PM, Mandy Chung wrote: On 10/23/18 10:34 PM, David Holmes wrote: On 24/10/2018 8:30 AM, dean.l...@oracle.com wrote: On 10/23/18 2:51 PM, David Holmes wrote: Hi Dean, On 24/10/2018 4:05 AM, dean.l...@oracle.com wrote: On 10/23/18 9:46 AM, dean.l...@oracle.com wrote: On

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-23 Thread dean . long
On 10/23/18 2:51 PM, David Holmes wrote: Hi Dean, On 24/10/2018 4:05 AM, dean.l...@oracle.com wrote: On 10/23/18 9:46 AM, dean.l...@oracle.com wrote: On 10/22/18 3:31 PM, David Holmes wrote: Sorry Dean I'm concerned about a thread termination bottleneck with this. A simple microbenchmark

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-23 Thread dean . long
On 10/23/18 9:46 AM, dean.l...@oracle.com wrote: On 10/22/18 3:31 PM, David Holmes wrote: Sorry Dean I'm concerned about a thread termination bottleneck with this. A simple microbenchmark that creates 500,000 threads that have to run and terminate, shows a 15+% slowdown on my Linux box. I

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-23 Thread dean . long
On 10/22/18 3:31 PM, David Holmes wrote: Sorry Dean I'm concerned about a thread termination bottleneck with this. A simple microbenchmark that creates 500,000 threads that have to run and terminate, shows a 15+% slowdown on my Linux box. I tried to find some kind of real benchmarks that

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-19 Thread dean . long
On 10/18/18 6:12 PM, Mandy Chung wrote: On 10/18/18 12:27 AM, David Holmes wrote: Hi Dean, On 18/10/2018 2:06 PM, dean.l...@oracle.com wrote: You're right, I missed that.  I think the right thing to do is call current_thread_exiting while holding the Threads_lock. Then we can get rid of

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
On 10/17/18 7:07 PM, David Holmes wrote: Hi Dean, This still seems racy to me. We increment all counts under the Threads_lock but we still decrement without the Threads_lock. So we can lose updates to the perfCounters.  117   _total_threads_count->inc();  118  

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
Thanks, Mandy! dl On 10/17/18 6:35 PM, Mandy Chung wrote: On 10/17/18 3:18 PM, dean.l...@oracle.com wrote: New webrevs, full and incremental: http://cr.openjdk.java.net/~dlong/8021335/webrev.6/ http://cr.openjdk.java.net/~dlong/8021335/webrev.6.diff/ I like it better without all the

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
Thanks JC! dl On 10/17/18 5:06 PM, JC Beyler wrote: Hi Dean, The new webrev looks much better :) LGTM (not a reviewer though :-)). Thanks, Jc On Wed, Oct 17, 2018 at 3:19 PM > wrote: On 10/17/18 2:38 PM, Mandy Chung wrote: On 10/17/18 2:13 PM,

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
On 10/17/18 2:38 PM, Mandy Chung wrote: On 10/17/18 2:13 PM, dean.l...@oracle.com wrote: On 10/17/18 1:41 PM, Mandy Chung wrote: On 10/16/18 7:33 PM, David Holmes wrote: Hi Dean, Thanks for tackling this. I'm still struggling to fully grasp why we need both the PerfCounters and the

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
On 10/17/18 1:41 PM, Mandy Chung wrote: On 10/16/18 7:33 PM, David Holmes wrote: Hi Dean, Thanks for tackling this. I'm still struggling to fully grasp why we need both the PerfCounters and the regular counters. I get that we have to decrement the live counts before ensure_join() has

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-17 Thread dean . long
On 10/16/18 7:33 PM, David Holmes wrote: Hi Dean, Thanks for tackling this. I'm still struggling to fully grasp why we need both the PerfCounters and the regular counters. I get that we have to decrement the live counts before ensure_join() has allowed Thread.join() to return, to ensure

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-16 Thread dean . long
New webrev: http://cr.openjdk.java.net/~dlong/8021335/webrev.4/ dl On 10/16/18 11:59 AM, dean.l...@oracle.com wrote: On 10/16/18 10:28 AM, JC Beyler wrote: Hi Dean, I noticed a typo here : "are atomically" is in the comment but should no longer be there I believe; the sentence probably

Re: RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-16 Thread dean . long
On 10/16/18 10:28 AM, JC Beyler wrote: Hi Dean, I noticed a typo here : "are atomically" is in the comment but should no longer be there I believe; the sentence probably should be: // These 2 counters are like the above thread counts, but are Fixed! Any way we could move this test into a

RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

2018-10-16 Thread dean . long
https://bugs.openjdk.java.net/browse/JDK-8021335 http://cr.openjdk.java.net/~dlong/8021335/webrev.3/ This change attempts to fix an old and intermittent problem with ThreadMXBean thread counts. Changes have 3 main parts: 1. Make sure hidden threads don't affect counts (even when exiting) 2.

Re: RFR: (S) 8210512: [Testbug] vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java fails with unexpected size of ClassLoaderReference.referringObjects

2018-09-10 Thread dean . long
+1 for this change +1 for learning new things :-) dl On 9/10/18 4:16 PM, serguei.spit...@oracle.com wrote: I also tried to learn from your email exchange with Dean L. Thanks, Serguei On 9/10/18 15:59, David Holmes wrote: Thanks for the reviews Serguei and JC. On 11/09/2018 8:10 AM, JC

Re: RFR (S) 8210512: vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java fails with unexpected size of ClassLoaderReference.referringObjects

2018-09-10 Thread dean . long
On 9/10/18 1:40 AM, David Holmes wrote: Hi Dean, On 10/09/2018 5:31 PM, dean.l...@oracle.com wrote: Hi David.  If a class references its own fields or methods, and that code is executed by the interpreter, then we should expect that constant pool entry to be resolved, shouldn't we? Yes

Re: RFR (S) 8210512: vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java fails with unexpected size of ClassLoaderReference.referringObjects

2018-09-10 Thread dean . long
Hi David.  If a class references its own fields or methods, and that code is executed by the interpreter, then we should expect that constant pool entry to be resolved, shouldn't we?  Likewise the compiler will treat it as resolved.  If we treat is as unresolved, we run into the case where we

Re: RFR(S): 8059334: nsk/jvmti/scenarios/hotswap/HS201/hs201t001 fails with exit code 0 after timeout

2017-11-06 Thread dean . long
OK thanks. dl On 11/6/17 10:56 AM, Chris Plummer wrote: Hi Dean, It looks like ciEnv::jvmti_state_changed() is used to support the JVMTI AddCapabilities() interface, which I believe typically a JVMTI agent uses to setup the available capabilities when the agent is first loaded (although

Re: RFR(S): 8059334: nsk/jvmti/scenarios/hotswap/HS201/hs201t001 fails with exit code 0 after timeout

2017-11-03 Thread dean . long
I'm not an expert in this area of code, but I'm wondering about Vladimir's comment about ciEnv::jvmti_state_changed() in the bug report.  With your fix, maybe we don't need to check ciEnv::jvmti_state_changed() (which doesn't seem to be enough by itself) and throw away the compiled result.  We

Re: RFR[9u-dev]: 8146683: check_addr0 should be more efficient

2016-02-23 Thread Dean Long
On 2/23/2016 10:07 AM, Daniel D. Daugherty wrote: On 2/23/16 5:56 AM, Thomas Stüfe wrote: Hi Cheleswer, On Tue, Feb 23, 2016 at 9:38 AM, Cheleswer Sahu wrote: Hi, Please review the code changes for https://bugs.openjdk.java.net/browse/JDK-8146683 .

Re: PING Re: RFR: 8078521: AARCH64: Add AArch64 SA support

2015-05-06 Thread Dean Long
I added hotspot-...@openjdk.java.net again. It looks reasonable to me, but I'm not a Reviewer. dl On 5/6/2015 7:36 AM, Andrew Haley wrote: On 04/29/2015 08:42 AM, Andrew Haley wrote: http://cr.openjdk.java.net/~aph/8078521-2/ Any news on this? It shouldn't be controversial at this point.

Re: RFR(S): 8073042: jcmd hangs until another jcmd is executed (which, in turn, also hangs) (on Windows)

2015-03-03 Thread Dean Long
, there is no blocking on the part of the AttachListener thread, WaitForSingleObject() return immediately decrementing the current count. Thanks Markus -Original Message- From: Dean Long Sent: den 3 mars 2015 02:32 To: Markus Gronlund; serviceability-dev@openjdk.java.net; hotspot-runtime

Re: RFR(S): 8073042: jcmd hangs until another jcmd is executed (which, in turn, also hangs) (on Windows)

2015-03-02 Thread Dean Long
Couldn't you instead treat it like a monitor? Then it's OK if only the first notify wakes up the blocked thread, as long as that thread only blocks when the queue is empty. In other words, when it wakes up, it should process all the items in the queue before blocking again. dl On 3/2/2015

Re: Request for approval: Backport of 8069030

2015-02-11 Thread Dean Long
Ping. Is there a different alias I should sent this to? dl On 2/9/2015 1:16 PM, Dean Long wrote: I would like to backport 8069030 to 8u60: https://bugs.openjdk.java.net/browse/JDK-8069030 http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/5960a65b0f54 The patch applied cleanly. dl

Re: Request for approval: Backport of 8069030

2015-02-11 Thread Dean Long
Thanks David. dl On 2/11/2015 3:33 PM, David Holmes wrote: I have no issue with this backport. David On 12/02/2015 7:00 AM, Dean Long wrote: Ping. Is there a different alias I should sent this to? dl On 2/9/2015 1:16 PM, Dean Long wrote: I would like to backport 8069030 to 8u60

Request for approval: Backport of 8069030

2015-02-09 Thread Dean Long
I would like to backport 8069030 to 8u60: https://bugs.openjdk.java.net/browse/JDK-8069030 http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/5960a65b0f54 The patch applied cleanly. dl

Re: RFR(XS) 8069030: support new PTRACE_GETREGSET

2015-02-02 Thread Dean Long
Thanks David and Staffan for the reviews. dl On 2/2/2015 5:07 PM, David Holmes wrote: Looks good. Thanks, David On 3/02/2015 8:46 AM, Dean Long wrote: Ping. I'm still waiting for a second review on this. thanks, dl On 1/28/2015 5:15 PM, Dean Long wrote: Adding serviceability-dev

Re: Review Request (S) 8025841: JVMTI: vtable stub dynamic code notification is misplaced

2014-02-07 Thread Dean Long
What's the cost for adding a new BufferBlob subtype? We already have AdapterBlob and MethodHandlesAdapterBlob. dl On 2/6/2014 3:52 PM, Oleg Mazurov wrote: My understanding was that a buffer blob was just that - a buffer. Could potentially contain code fragments of different kinds. Thus,

Re: Review Request (S) 8025841: JVMTI: vtable stub dynamic code notification is misplaced

2014-02-07 Thread Dean Long
code. In fact, I was going to file a linked JIRA issue on that further improvement and if my idea for it holds true there would be no need for a new BufferBlob subtype. -- Oleg On 2/7/2014 1:06 PM, Dean Long wrote: What's the cost for adding a new BufferBlob subtype? We already have

Re: RFR 8003421: NPG: Move oops out of InstanceKlass into mirror

2013-05-20 Thread Dean Long
It seems like you could take this opportunity to make these declared fields of java.lang.Class, allowing, for example, getProtectionDomain0() to be a simple Java method instead of a native method. dl On 05/20/2013 03:39 PM, Coleen Phillimore wrote: Summary: Inject protection_domain, signers,

Re: RFR 8003421: NPG: Move oops out of InstanceKlass into mirror

2013-05-20 Thread Dean Long
On 5/20/2013 7:50 PM, Coleen Phillimore wrote: On 5/20/2013 8:42 PM, Dean Long wrote: It seems like you could take this opportunity to make these declared fields of java.lang.Class, allowing, for example, getProtectionDomain0() to be a simple Java method instead of a native method. We

Re: RFR 8003421: NPG: Move oops out of InstanceKlass into mirror

2013-05-20 Thread Dean Long
Right, unless there is a way to inject them conditionally. dl On 5/20/2013 9:02 PM, Ioi Lam wrote: But if you move these fields into Class.java (in JDK8), then hsx25 will not run on JDK7 anymore, unless these fields are also added in Class.java in JDK7. - Ioi On 05/20/2013 05:42 PM, Dean

Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c

2013-03-05 Thread Dean Long
What if allocate_init_map() fails to allocate memory? Shouldn't add_class_share_map_info() follow the pattern of add_map_info(): return NULL or map, and have the caller check for NULL? dl On 3/4/2013 11:39 PM, David Holmes wrote: Looks fine to me - thanks Staffan! David On 5/03/2013 5:24

Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c

2013-03-05 Thread Dean Long
On 3/5/2013 3:23 PM, David Holmes wrote: On 6/03/2013 9:08 AM, Dean Long wrote: What if allocate_init_map() fails to allocate memory? Shouldn't add_class_share_map_info() follow the pattern of add_map_info(): return NULL or map, and have the caller check for NULL? AFAICS apart from one

Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c

2013-03-05 Thread Dean Long
On 3/5/2013 4:48 PM, David Holmes wrote: On 6/03/2013 10:44 AM, Dean Long wrote: On 3/5/2013 3:23 PM, David Holmes wrote: On 6/03/2013 9:08 AM, Dean Long wrote: What if allocate_init_map() fails to allocate memory? Shouldn't add_class_share_map_info() follow the pattern of add_map_info

Re: RFR(XXS): 8007147: Trace event ExecuteVMOperation may get dangling pointer

2013-02-19 Thread Dean Long
On 2/19/2013 11:00 AM, Staffan Larsen wrote: On 19 feb 2013, at 19:56, Dean Long dean.l...@oracle.com mailto:dean.l...@oracle.com wrote: When the VM_Operation is created, we could take a snapshot of the thread_id of the caller, then use that later. One problem

Re: RFR: 7152800: All tests using the attach API fail with well-known file is not secure on Mac OS X

2012-03-20 Thread Dean Long
2012, at 20:38, Dean Long wrote: On linux, you can control this behavior with the setgid mode bit on the parent directory. Does that also work on OSX? dl On 3/19/2012 6:03 AM, Staffan Larsen wrote: Please review the following fix: Webrev: http://cr.openjdk.java.net/~sla/7152800/webrev.00

Re: RFR: 7152800: All tests using the attach API fail with well-known file is not secure on Mac OS X

2012-03-19 Thread Dean Long
On linux, you can control this behavior with the setgid mode bit on the parent directory. Does that also work on OSX? dl On 3/19/2012 6:03 AM, Staffan Larsen wrote: Please review the following fix: Webrev: http://cr.openjdk.java.net/~sla/7152800/webrev.00/

Auto Reply: serviceability-dev Digest, Vol 48, Issue 5

2011-11-04 Thread dean . long
This is an auto-replied message. I am out of the office right now.