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

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer-2
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
Reply | Threaded
Open this post in threaded view
|

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

Serguei Spitsyn-2
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
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer-2
In reply to this post by Chris Plummer-2
> 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.

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

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
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer-2
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
Reply | Threaded
Open this post in threaded view
|

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

Kevin Walls-2
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
Reply | Threaded
Open this post in threaded view
|

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

Kevin Walls-2
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
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer-2
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
Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer-2
In reply to this post by Chris Plummer-2
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

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