Hi Ralf,
A few comments below, but overall looks good:
27 * @summary get stack trace for large stacks took too long.
How about "Test that getting the stack trace for a very large stack does
not take too long".
The max number of frames you'll test for is 100M, but the stack size is
set to 4m, assuming -Xss works (and I think on some platforms it may
not). 100M frames seems like overkill for a 4M stack. If the stack was
nothing more than a frame link pointer on a 32-bit system, you'd only
have 1M frames, but lets be more realistic than that and say you should
never have more than 256k frames. Lowering the max number of frames will
prevent this test from taking a very long time on platforms where -Xss
has failed.
65 // Have some frames be removed before we call again.
Should this be: "Pop some frames so there is room on the stack for the
println()"
96 bpe = resumeTo("Frames2Targ", "callEnded", "()V");
What happens if we never get to callEnded()?
thanks,
Chris
On 7/17/18 7:08 AM, Schmelter, Ralf wrote:
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-boun...@openjdk.java.net]
On Behalf Of Schmelter, Ralf
Sent: Montag, 9. Juli 2018 16:05
To: Chris Plummer <chris.plum...@oracle.com>;
serviceability-dev@openjdk.java.net; serguei.spit...@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.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