RFR (S) 8205608: Fix 'frames()' in ThreadReferenceImpl.c to prevent quadratic runtime behavior

[Schmelter, Ralf]

Hi all,

here is an updated webref at http://cr.openjdk.java.net/~simonis/webrevs/2018/8205608.v2/ 

I've converted the shell based test to a Java one, which had the nice side effect of speeding it up (now takes ~1 second runtime).

The fix itself is mainly unchanged, but I've added the variable 'filledIn' to store the number of frames actually filled in by the GetStackTrace call. Formerly I've reused the count variable, but this can lead to misunderstandings.

Best regards,
Ralf


-----Original Message-----
From: serviceability-dev [mailto:serviceability-dev-bounces at openjdk.java.net] On Behalf Of Schmelter, Ralf
Sent: Montag, 9. Juli 2018 16:05
To: Chris Plummer <chris.plummer at oracle.com>; serviceability-dev at openjdk.java.net; serguei.spitsyn at oracle.com
Subject: [CAUTION] RE: RFR (S) 8205608: Fix 'frames()' in ThreadReferenceImpl.c to prevent quadratic runtime behavior

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.plummer at oracle.com] 
Sent: Freitag, 6. Juli 2018 00:37
To: Schmelter, Ralf <ralf.schmelter at sap.com>; serviceability-dev at openjdk.java.net; serguei.spitsyn at 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