Hi Nick,
Adding to Andrew comments, maybe the solution is to have the test extend
LingeredApp so it can produce a more reliable stack trace other than the
default one you get with LingeredApp. If that's too much trouble, I
don't mind the solution you came up with, but seems writing a
LingeredApp subclass that is specific for this test would be cleaner.
thanks,
Chris
On 8/12/19 3:24 AM, Nick Gasson wrote:
Thanks Andrew. Can someone from the serviceability team check this is
OK to push?
Nick
On 08/08/2019 18:16, Andrew Dinn wrote:
Hi Nick,
On 08/08/2019 10:32, Nick Gasson wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8229118
Webrev: http://cr.openjdk.java.net/~ngasson/8229118/webrev.0/
This test starts a sub-process with -Xcomp and then uses the SA to
get a
stack trace of it. It expects to see this line:
In code in NMethod for jdk/test/lib/apps/LingeredApp.main
But actually on AArch64 the stack trace looks like this:
- java.lang.Thread.sleep(long) @bci=0, pc=0x0000ffff74603d08,
Method*=0x0000ffff031baf98 (Compiled frame; information may be
imprecise)
- jdk.test.lib.apps.LingeredApp.main(java.lang.String[]) @bci=53,
line=502, pc=0x0000ffff6c9276e0, Method*=0x0000ffff03611d48
(Interpreted frame)
The main method is interpreted even though we're running with
-Xcomp. That's because it is deoptimized almost immediately, because
main calls some methods on java.nio.file.Paths, but that class hasn't
been loaded when main is compiled.
X86 can patch in the address of the method on-the-fly, but AArch64
can't
do this because of restrictions on which instructions can be legally
rewritten.
This patch lifts the code that uses the java.nio classes out of
LingeredApp::main into a separate static method. LingeredApp.main now
only uses classes that are loaded very early in boot, before main is
compiled. The stack trace now looks like:
"main" #1 prio=5 tid=0x0000ffffb4022800 nid=0xd610 waiting on
condition [0x0000ffffbb755000]
java.lang.Thread.State: TIMED_WAITING (sleeping)
JavaThread state: _thread_blocked
- java.lang.Thread.sleep(long) @bci=0, pc=0x0000fffface414c8,
Method*=0x0000ffff3dac8a28 (Compiled frame; information may be
imprecise)
- jdk.test.lib.apps.LingeredApp.pollLockFile(java.lang.String)
@bci=30, line=499, pc=0x0000ffffa50818e0, Method*=0x0000ffff3c122cf0
(Interpreted frame)
- jdk.test.lib.apps.LingeredApp.main(java.lang.String[]) @bci=25,
line=529, pc=0x0000ffffa59afd58, Method*=0x0000ffff3c122de0
(Compiled frame)
I.e. pollLockFile was deoptimized to an interpreted frame but
LingeredApp.main is still a compiled frame which is what
ClhsdbFindPC is
looking for.
This solution does seem a bit hacky, so if it's not acceptable an
alternative is to just skip the -Xcomp part of the test on AArch64.
Ran a full jtreg test on AArch64/x86 to check for regressions.
Yuck! That's a nice hack to avoid the indeterminate effect of -Xcomp.
However, my gut feeling is still that relying on -Xcomp in tests is just
a /really/ bad idea and I'd prefer to omit it but . . .
I'm not 100% clear what the point of this test is but it looks like it
is meant to exercise the stack backtrace code when there is a compiled
method on the stack. If so then I guess your hack fits the bill while
removing the -Xcomp flag from the command line would not fulfil the
test's remit. If that is the point of the test then I agree,
reluctantly, that your hack is the right solution. On those grounds I'm
happy to accept the patch. However, I'd prefer someone else (Andrew
Haley?) also to review this before it gets pushed.
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
