OpenJDK › OpenJDK Serviceability Development

RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice

Classic

List

Threaded

8 messages Options

Chris Plummer-2

RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice

See CR for details. In brief, fixed the `inspect` command to remove duplicate output:

hsdb> inspect 0x00000007fef00770
instance of Oop for java/lang/Class @ 0x00000007fef00770 @ 0x00000007fef00770 (size = 160)
in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08 Oop for java/io/BufferedInputStream @ 0x0000000082005b08
out: Oop for java/io/PrintStream @ 0x0000000082007b60 Oop for java/io/PrintStream @ 0x0000000082007b60
err: Oop for java/io/PrintStream @ 0x000000008200e0c8 Oop for java/io/PrintStream @ 0x000000008200e0c8
It should be:

hsdb> inspect 0x00000007fef00770
instance of Oop for java/lang/Class @ 0x00000007fef00770 (size = 160)
in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08
out: Oop for java/io/PrintStream @ 0x0000000082007b60
err: Oop for java/io/PrintStream @ 0x000000008200e0c8

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

Commit messages:
- Fixed some comments.
- Fixed some comments.
- Fixed inspect command so it doesn't duplicate some output

Changes: https://git.openjdk.java.net/jdk/pull/2582/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2582&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8261269
Stats: 116 lines in 2 files changed: 90 ins; 22 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/2582.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2582/head:pull/2582

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

Serguei Spitsyn-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice

On Tue, 16 Feb 2021 07:28:45 GMT, Chris Plummer <[hidden email]> wrote:

> See CR for details. In brief, fixed the `inspect` command to remove duplicate output:
>
> hsdb> inspect 0x00000007fef00770
> instance of Oop for java/lang/Class @ 0x00000007fef00770 @ 0x00000007fef00770 (size = 160)
> in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08 Oop for java/io/BufferedInputStream @ 0x0000000082005b08
> out: Oop for java/io/PrintStream @ 0x0000000082007b60 Oop for java/io/PrintStream @ 0x0000000082007b60
> err: Oop for java/io/PrintStream @ 0x000000008200e0c8 Oop for java/io/PrintStream @ 0x000000008200e0c8
> It should be:
>
> hsdb> inspect 0x00000007fef00770
> instance of Oop for java/lang/Class @ 0x00000007fef00770 (size = 160)
> in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08
> out: Oop for java/io/PrintStream @ 0x0000000082007b60
> err: Oop for java/io/PrintStream @ 0x000000008200e0c8

It looks good to me.
One side comment about the test. It is not easy to read the code with the same pattern (e.g. "Oop for java/io/BufferedInputStream") repeated several times. I understand, you prefer to make it more explicit but it'd be more readable if a predefined pattern is used like below:
String pattern = "Oop for java/io/BufferedInputStream";
+ expStrMap.put(cmd, List.of("instance of Oop for java/lang/Class @ " + examineResult,
+ "in: " + pattern + @"));
. . .
+ unexpStrMap.put(cmd, List.of(
+ "instance of Oop for java/lang/Class @ " + examineResult + " @ " + examineResult,
+ "in: " + pattern + " .* " + pattern));

Just an opinion with no pressure. It is up to you.

Thanks,
Serguei

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

Marked as reviewed by sspitsyn (Reviewer).

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

Chris Plummer-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice [v2]

In reply to this post by Chris Plummer-2

Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:

Put replicated strings into a local variable.

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

Changes:
- all: https://git.openjdk.java.net/jdk/pull/2582/files
- new: https://git.openjdk.java.net/jdk/pull/2582/files/d5a300c3..1304bd78

Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2582&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2582&range=00-01

Stats: 8 lines in 1 file changed: 2 ins; 0 del; 6 mod
Patch: https://git.openjdk.java.net/jdk/pull/2582.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2582/head:pull/2582

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

Chris Plummer-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice [v2]

In reply to this post by Serguei Spitsyn-2

On Wed, 17 Feb 2021 23:30:04 GMT, Serguei Spitsyn <[hidden email]> wrote:

>> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Put replicated strings into a local variable.
>
> It looks good to me.
> One side comment about the test. It is not easy to read the code with the same pattern (e.g. "Oop for java/io/BufferedInputStream") repeated several times. I understand, you prefer to make it more explicit but it'd be more readable if a predefined pattern is used like below:
> String pattern = "Oop for java/io/BufferedInputStream";
> + expStrMap.put(cmd, List.of("instance of Oop for java/lang/Class @ " + examineResult,
> + "in: " + pattern + @"));
> . . .
> + unexpStrMap.put(cmd, List.of(
> + "instance of Oop for java/lang/Class @ " + examineResult + " @ " + examineResult,
> + "in: " + pattern + " .* " + pattern));
>
> Just an opinion with no pressure. It is up to you.
>
> Thanks,
> Serguei

Good idea. I've added the pattern strings. Thanks!

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

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

Kevin Walls-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice [v2]

In reply to this post by Chris Plummer-2

On Thu, 18 Feb 2021 06:43:01 GMT, Chris Plummer <[hidden email]> wrote:

>> See CR for details. In brief, fixed the `inspect` command to remove duplicate output:
>>
>> hsdb> inspect 0x00000007fef00770
>> instance of Oop for java/lang/Class @ 0x00000007fef00770 @ 0x00000007fef00770 (size = 160)
>> in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08 Oop for java/io/BufferedInputStream @ 0x0000000082005b08
>> out: Oop for java/io/PrintStream @ 0x0000000082007b60 Oop for java/io/PrintStream @ 0x0000000082007b60
>> err: Oop for java/io/PrintStream @ 0x000000008200e0c8 Oop for java/io/PrintStream @ 0x000000008200e0c8
>> It should be:
>>
>> hsdb> inspect 0x00000007fef00770
>> instance of Oop for java/lang/Class @ 0x00000007fef00770 (size = 160)
>> in: Oop for java/io/BufferedInputStream @ 0x0000000082005b08
>> out: Oop for java/io/PrintStream @ 0x0000000082007b60
>> err: Oop for java/io/PrintStream @ 0x000000008200e0c8
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>
> Put replicated strings into a local variable.

Marked as reviewed by kevinw (Committer).

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

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

Kevin Walls-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice [v2]

In reply to this post by Chris Plummer-2

On Thu, 18 Feb 2021 06:43:04 GMT, Chris Plummer <[hidden email]> wrote:

>> It looks good to me.
>> One side comment about the test. It is not easy to read the code with the same pattern (e.g. "Oop for java/io/BufferedInputStream") repeated several times. I understand, you prefer to make it more explicit but it'd be more readable if a predefined pattern is used like below:
>> String pattern = "Oop for java/io/BufferedInputStream";
>> + expStrMap.put(cmd, List.of("instance of Oop for java/lang/Class @ " + examineResult,
>> + "in: " + pattern + @"));
>> . . .
>> + unexpStrMap.put(cmd, List.of(
>> + "instance of Oop for java/lang/Class @ " + examineResult + " @ " + examineResult,
>> + "in: " + pattern + " .* " + pattern));
>>
>> Just an opinion with no pressure. It is up to you.
>>
>> Thanks,
>> Serguei
>
> Good idea. I've added the pattern strings. Thanks!

That's great, much more readable output!
Looks good.

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

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

Chris Plummer-2

Re: RFR: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice [v2]

In reply to this post by Kevin Walls-2

On Mon, 22 Feb 2021 12:05:20 GMT, Kevin Walls <[hidden email]> wrote:

>> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Put replicated strings into a local variable.
>
> Marked as reviewed by kevinw (Committer).

Thanks Serguei and Kevin!

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

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

Chris Plummer-2

Integrated: 8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice

In reply to this post by Chris Plummer-2

On Tue, 16 Feb 2021 07:28:45 GMT, Chris Plummer <[hidden email]> wrote:

This pull request has now been integrated.

Changeset: aea474c4
Author: Chris Plummer <[hidden email]>
URL: https://git.openjdk.java.net/jdk/commit/aea474c4
Stats: 117 lines in 2 files changed: 92 ins; 22 del; 3 mod

8261269: When using clhsdb to "inspect" a java object, clhsdb prints "Oop for..." twice

Reviewed-by: sspitsyn, kevinw

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

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