On Wed, 10 Mar 2021 03:48:29 GMT, Vipin Sharma <vsha...@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
