RFR JDK-8215425: vmTestbase/nsk/jvmti/PopFrame should provide more detailed output
JC Beyler
jcbeyler at google.com
Wed Dec 19 03:35:21 UTC 2018
Hi Alex,
Looks good to me as well :)
Nice job!
Jc
On Tue, Dec 18, 2018 at 6:10 PM <serguei.spitsyn at oracle.com> wrote:
> Alex,
>
> Thank you for the update!
>
> It looks good.
> There is another instance with incorrect spacing:
>
> 121 totRes=doPopFrame(false, Thread.currentThread());
>
>
> No need in new webrev if you fix the above.
>
> Thanks,
> Serguei
>
>
> On 12/18/18 6:01 PM, Alex Menkov wrote:
>
> Hi Serguei,
>
> Thank you for the review.
> Updated webrev:
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.02/
>
> On 12/18/2018 16:49, serguei.spitsyn at oracle.com wrote:
>
> Hi Alex,
>
> A couple of minor comments.
>
>
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popframe002.java.frames.html
>
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popframe004.java.frames.html
>
> While you are at these files, could you, fix several originally ugly
> indented comments?
>
>
> Done.
>
>
> Could you, also, fix the incorrect spacing around '=' in the
> popframe004.java:
>
> 95 retValue=doPopFrame(true, popFrameClsThr);
>
>
> Done.
>
> Also fixed comment in popframe004/TestDescription.java (to be in sync with
> comment change in popframe004.java)
>
>
>
> Could you give an idea about the motivation to remove the following
> fragment ?:
>
>
> This is "test case 2" in popframe004 which I mentioned in the review
> request.
> The code is never executed (if it is, this means the test has already
> failed) and I don't have an idea what other case can be tested here.
>
> --alex
>
>
> 167 if (popframe004.popFdone) { // popping has been done
> 168 if (DEBUG_MODE)
> 169 out.println("popFrameCls (" + this +
> 170 "): enter activeMethod() after popping");
> 171 // test case #2: popping from the current thread
> 172 if (!popframe004.popF2done) {
> 173 popframe004.popF2done = true;
> 174 if (DEBUG_MODE) {
> 175 out.println("popFrameCls (" + this +
> 176 "): going to pop a frame from the current thread...");
> 177 retVal = doPopFrame(3, popFrameClsThr);
> 178 } else
> 179 retVal = doPopFrame(2, popFrameClsThr);
> 180 if (retVal != PASSED)
> 181 popframe004.totRes = FAILED;
> 182 }
> 183 if (DEBUG_MODE)
> 184 out.println("popFrameCls (" + this +
> 185 "): leaving activeMethod()...");
> 186 return;
> 187 }
>
>
> Otherwise, it looks good.
>
> Thanks,
> Serguei
>
>
> On 12/18/18 1:37 PM, Alex Menkov wrote:
>
> Hi all,
>
> please review the fix for
> https://bugs.openjdk.java.net/browse/JDK-8215425
> webrev:
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/
>
> The fix
> - turns on detailed output for the tests and cleaned related stuff;
> - for popframe002 deletes output from run() method as it caused unexpected
> MethodExit event;
> - removes "test case 2" in popframe004 test as it's never executed.
>
> --alex
>
>
>
>
--
Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181218/d48a9d91/attachment.html>
More information about the serviceability-dev
mailing list