Hi Chris, thanks for the review.
> What testing have you done? I've tested the change by debugging by hand in eclipse and jdb, running the com/sun/jdi rtreg tests and the jdwp jck tests. And analog code is running in the SAP JVM for many years. > How long does this test take to run. 15 s according to jtreg. > What happens if for some reason SOE is never thrown? It's not clear to > me what the script would do in this case. It is treated as passed (which is not ideal). > In answer to the ShellScaffold.sh question, there is already work > underway to convert to pure java tests. See JDK-8201652. Ok, then I think it is better to convert the test to a Java TestScaffold test. I will update the webref when this is done. Best regards, Ralf -----Original Message----- From: Chris Plummer [mailto:chris.plum...@oracle.com] Sent: Freitag, 6. Juli 2018 00:37 To: Schmelter, Ralf <ralf.schmel...@sap.com>; serviceability-dev@openjdk.java.net; serguei.spit...@oracle.com Subject: Re: RFR (S) 8205608: Fix 'frames()' in ThreadReferenceImpl.c to prevent quadratic runtime behavior Hi Ralf, Overall looks good, but I do have a few comments and questions. Please update the copyright. What testing have you done? How long does this test take to run. What happens if for some reason SOE is never thrown? It's not clear to me what the script would do in this case. In answer to the ShellScaffold.sh question, there is already work underway to convert to pure java tests. See JDK-8201652. I'm not certain if it is ok for you to just submit this new shell script, or if should be rewritten in pure java. Most of the work to convert the scripts has already been done but was put on hold. Maybe Serguei can comment and guide you on how it would be done in java. thanks, Chris On 7/3/18 3:43 AM, Schmelter, Ralf wrote: > Hi All, > > Please review the fix for the > bughttps://bugs.openjdk.java.net/browse/JDK-8205608 . The webref is > athttp://cr.openjdk.java.net/~simonis/webrevs/2018/8205608/ . > > This fixes the quadratic runtime (in the number of frames) of the frames() > method, making it linear instead. It uses additional memory proportional to > the number of frames on the stack. > > I've included a jtreg test, which would time out in the old implementation > (since it takes minutes to get the stack frames). I'm not sure how useful > this is. > > Best regards, > Ralf Schmelter
