On Sun, 7 Mar 2021 10:43:34 GMT, Vipin Sharma <vsha...@openjdk.org> wrote:
> JDK-8261095: Add test for clhsdb "symbol" command
Overall it looks good. I have some suggestions for improved comments and
formatting, and also please use the line.separator property.
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 54:
> 52: System.out.println("Started LingeredApp with pid " +
> theApp.getPid());
> 53:
> 54: //Use command "class java.lang.Thread" to get the address of
> the InstanceKlass for java.lang.Thread
Put a space after the `//`. Same goes for all the comments below.
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 85:
> 83:
> 84: //extract address comes along with Symbol instance, following
> is corresponding sample output line
> 85: //Symbol* Klass::_name: Symbol @ 0x0000000800471120
// The inspect command output will have one line of output that looks like the
following.
// Symbol* Klass::_name: Symbol @ 0x0000000800471120
// Extract the Symbol address from it.
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 97:
> 95: }
> 96:
> 97: //Running "symbol" command on the Symbol instance address
> extracted in previous step
// Run "symbol" command on the Symbol instance address extracted in previous
step.
// It should produce the symbol for java/lang/Thread.
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 103:
> 101: expStrMap.put(cmdStr, List.of("#java/lang/Thread"));
> 102: test.run(theApp.getPid(), cmds, expStrMap, null);
> 103:
No need for newline here.
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 63:
> 61: String[] parts = classOutput.split("\n");
> 62:
> 63: //extract thread address from the output line similar to
> "java/lang/Thread @0x000000080001d940"
I'd prefer:
// Extract the thread InstanceKlass address from the output, which looks
similar to the following:
// java/lang/Thread @0x000000080001d940
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 61:
> 59: String classOutput = test.run(theApp.getPid(), cmds,
> expStrMap, null);
> 60: String threadAddress = null;
> 61: String[] parts = classOutput.split("\n");
I think you should be using the line.separator properly instead of "\n".
` String linesep = System.getProperty("line.separator");`
test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 82:
> 80: String inspectOutput = test.run(theApp.getPid(), cmds,
> expStrMap, null);
> 81: String symbolAddress = null;
> 82: parts = inspectOutput.split("\n");
And here also use line.separator.
-------------
Changes requested by cjplummer (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2863
