RFR: JDK-8261095: Add test for clhsdb "symbol" command

Chris Plummer cjplummer at openjdk.java.net
Mon Mar 8 23:52:09 UTC 2021


On Sun, 7 Mar 2021 10:43:34 GMT, Vipin Sharma <vsharma at 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


More information about the serviceability-dev mailing list