Re: RFR (L) 8210429: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[G-Z] tests

2018-09-05 Thread Chris Plummer
Hi JC, In getstacktr008.cpp, this should be one line:  240 classDef.class_byte_count =  241 env->GetArrayLength(classBytes); Otherwise looks great! Chris On 9/5/18 6:39 PM, JC Beyler wrote:

RFR (L) 8210429: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[G-Z] tests

2018-09-05 Thread JC Beyler
Hi all, Continuing the removal of the JNI_ENV* macros, I have done the other half of the JVMTI Get[G-Z] methods. The final JVMTI test refactoring will come in a final webrev after this one. The change is straightforward as before, just a bit repetitive: Webrev: http://cr.openjdk.java.net/~jcbeyle

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

2018-09-05 Thread Igor Ignatyev
Hi JC, > On Sep 5, 2018, at 2:59 PM, JC Beyler wrote: > > Hi Igor, > > I like this much better! A few more comments: > > - > http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html > >

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

2018-09-05 Thread Igor Ignatyev
Hi JC, thanks for reviewing this! I agree w/ both your comments and have updated the code very similarly to your suggestion. I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine method family is a bit different from other should* (and strange), besides checking that the lines match

Re: RFR (M) 8210198: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[A-F] tests

2018-09-05 Thread Chris Plummer
Hi JC, Looks good. For the bytecodes001.cpp formatting, I just thought it odd you put a two char argument on the same line and the rest on the new line. All on the same line seemed better, either as you have changed it to or all on the next lin

Re: RFR (S) 8208352: Merge HeapMonitorTest and HeapMonitorGCTest code

2018-09-05 Thread Alex Menkov
+1 --alex On 09/05/2018 12:32, serguei.spit...@oracle.com wrote: Hi Jc, +1 Thanks, Serguei On 9/5/18 11:59, Chris Plummer wrote: Looks good. Chris On 9/5/18 11:14 AM, JC Beyler wrote: Hi all, Two of the tests shared code and we could refactor to limit future fixes/changes in both spot

Re: RFR (M) 8210198: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[A-F] tests

2018-09-05 Thread JC Beyler
Hi Chris, I had seen that formatting in bytecodes001.cpp but was not sure it made the line a bit too long (I think there is no real line wrap limit, just what seems reasonable, correct?). But done since you saw it too, that makes two of us wanting to change it! I changed all the IsSameObject != J

Re: RFR (S) 8208352: Merge HeapMonitorTest and HeapMonitorGCTest code

2018-09-05 Thread serguei.spit...@oracle.com
Hi Jc, +1 Thanks, Serguei On 9/5/18 11:59, Chris Plummer wrote: Looks good. Chris On 9/5/18 11:14 AM, JC Beyler wrote: Hi all,

Re: RFR (M) 8210198: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[A-F] tests

2018-09-05 Thread Chris Plummer
Hi JC, In bytecodes001.cpp the formatting could use some improvement:  130 if (meth_tab[meth_ind].stat == JNI_TRUE) {  131 mid = env->GetStaticMethodID(cl,  132 meth_tab[meth_ind].name, meth_tab[meth_ind].sig);  133

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Gary Adams
After a quick local check of finishedThreads =1, I moved the check til after the full list of threads is checked, rather than repeating the stderr failure after each entry was processed. I also implemented a resumeVM() in TestScaffold and called it before dumpThreads(). That did not fail locally,

Re: RFR (M) 8210198: Clean up JNI_ENV_ARG for vmTestbase/jvmti/Get[A-F] tests

2018-09-05 Thread Alex Menkov
+1 --alex On 09/04/2018 17:10, serguei.spit...@oracle.com wrote: Hi Jc, It looks good to me. Thank you for this cleanup! Thanks, Serguei On 9/4/18 16:12, JC Beyler wrote: Hi all, Continuing the removal of the JNI_ENV* macros, I have done about half of the JVMTI Get[A-F] methods. The whol

Re: RFR (S) 8208352: Merge HeapMonitorTest and HeapMonitorGCTest code

2018-09-05 Thread Chris Plummer
Looks good. Chris On 9/5/18 11:14 AM, JC Beyler wrote: Hi all, Two of the tests shared code and we could refactor to limit future fixes/changes in both spots. Here is

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Chris Plummer
I was thinking something like the resume() call. At the very least just try setting finishedThreads = 1 to exercise the error handling. Chris  On 9/5/18 11:49 AM, Gary Adams wrote: I had tried sleeping after each entry in dumpThreads was checked, but the failure never occurred. I had tried us

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Gary Adams
I had tried sleeping after each entry in dumpThreads was checked, but the failure never occurred. I had tried using mach5 --jvm-args to set UseZGC, but that never failed. Could attempt a resume() before calling dumpThreads() - haven't tried that , yet. On 9/5/18, 2:31 PM, Chris Plummer wrote:

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Chris Plummer
Ok. Have you found a way to test your changes? Maybe just force or simulate a failure somehow. Chris On 9/5/18 11:26 AM, Gary Adams wrote: If we go ahead with this proposed fix, the next time it fails we will have more information about the other threads captured in the log. One last observat

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Gary Adams
If we go ahead with this proposed fix, the next time it fails we will have more information about the other threads captured in the log. One last observation, the test that failed was running with UseZGC. Not sure how that might impact this particular test. On 9/5/18, 2:07 PM, Chris Plummer wrot

RFR (S) 8208352: Merge HeapMonitorTest and HeapMonitorGCTest code

2018-09-05 Thread JC Beyler
Hi all, Two of the tests shared code and we could refactor to limit future fixes/changes in both spots. Here is the small webrev that does that: Webrev: http://cr.openjdk.java.net/~jcbeyler/8208352/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8208352 Thank you for the future reviews,

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Chris Plummer
Hi Gary, If the DebugThreadTarg is resumed prematurely, The failure explanation seems to hinge on this, but I don't see how this can happen. What is resuming DebugThreadTarg? Only the call to listenUntilVMDisconnect() should be doing this, and we aren't even getting that far. The only thin

Re: Is it possible to resurrect sa-jdi?

2018-09-05 Thread Bernd Eckenfels
I thought by providing jhsdb launcher the usage of dump based analysis became more prominent, so those attach mechanisms are not going away, right? -- https://Bernd.eckenfels.net Von: -800456320m Auftrag von Gesendet: Mittwoch, September 5, 2018 1:17 PM An: Egor

Re: RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

2018-09-05 Thread Gary Adams
The DebuggerThreadTest ensures it is in sync at the beginning of RunTests with a breakpoint event in DebuggerThreadTarg ready() method. The DebuggerThreadTest then continues with dumpThreads() and listenUntilVMDisconnect() completes. If the DebugThreadTarg is resumed prematurely, the main thread

Re: Is it possible to resurrect sa-jdi?

2018-09-05 Thread Egor Ushakov
Hi Alan, this is not yet released in IDEA, so no usage for now. However it looks much better than the usual thread dump, so if it could be used easily (and we're trying to achieve that) it could easily replace the thread dump action for processes in IDEA. On 05-Sep-18 13:47, Alan Bateman wrot

Re: Is it possible to resurrect sa-jdi?

2018-09-05 Thread Alan Bateman
On 05/09/2018 11:25, Egor Ushakov wrote: Hi, suddenly at Jetbrains we realized that sa pid attach may be useful. Unfortunately sun.jvm.hotspot.jdi* was removed in jdk 9. We would like to resurrect sa-jdi in our jdk fork. As I can see the agent is still there, so it should be feasible. Do you s

Is it possible to resurrect sa-jdi?

2018-09-05 Thread Egor Ushakov
Hi, suddenly at Jetbrains we realized that sa pid attach may be useful. Unfortunately sun.jvm.hotspot.jdi* was removed in jdk 9. We would like to resurrect sa-jdi in our jdk fork. As I can see the agent is still there, so it should be feasible. Do you see any issues with that? Should we go in t