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

Chris Plummer cjplummer at openjdk.java.net
Wed Mar 10 04:36:14 UTC 2021


On Wed, 10 Mar 2021 03:48:29 GMT, Vipin Sharma <vsharma at openjdk.org> wrote:

>> JDK-8261095: Add test for clhsdb "symbol" command
>
> Vipin Sharma has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Started using orElseThrow and removed null check following this

I didn't realize that `lines()` returns a `Stream`. TBH I find the original code a lot easier to read than the `Stream` code, but that's pretty much always my opinion when I see `Stream` code. Others will probably feel your new version is more elegant, so I won't protest.

Having said that, I also just learned that `String.split("\R")` will properly split the lines with any newline character sequence. See [Pattern](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/regex/Pattern.html):
`\R    Any Unicode linebreak sequence \u000D\u000A|[\u000A\u000B\u000C\u000D\u0085\u2028\u2029] `

test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 68:

> 66:                                                       .map(addresses -> addresses[1])
> 67:                                                       .orElseThrow(() -> new RuntimeException("Cannot find address of " +
> 68:                                                               "the InstanceKlass for java.lang.Thread in output"));

These lines really don't format well, a reason why the previous `orElse()` version is probably better. Maybe you can make this work by moving the `.filter()` call to a new line that is indented 8 more than the previous line

test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 71:

> 69: 
> 70: 
> 71:             // Use "inspect" on the thread address we extracted in previous step

"in `the` previous step"

test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 79:

> 77: 
> 78:             // The inspect command output will have one line of output that looks like the following.
> 79:             // Symbol* Klass::_name: Symbol @ 0x0000000800471120

Indent this line of the comment

test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 87:

> 85:                                                                 "Cannot find address with Symbol instance"));
> 86: 
> 87:             // Run "symbol" command on the Symbol instance address extracted in previous step.

"in `the` previous step"

test/hotspot/jtreg/serviceability/sa/ClhsdbSymbol.java line 85:

> 83:                                                         .findFirst().map(symbolParts -> symbolParts[1])
> 84:                                                         .orElseThrow(() -> new RuntimeException(
> 85:                                                                 "Cannot find address with Symbol instance"));

Same issue with formatting as above.

-------------

Changes requested by cjplummer (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2863


More information about the serviceability-dev mailing list