RFR: CODETOOLS-7903176: Move LJV inside JOL [v5]

Aleksey Shipilev shade at openjdk.org
Wed Oct 12 13:20:32 UTC 2022


On Mon, 26 Sep 2022 15:50:13 GMT, Ivan Ponomarev <duke at openjdk.org> wrote:

>> This moves https://github.com/atp-mipt/ljv project inside JOL in order for LJV to get access to internal JOL's APIs.
>> 
>> See blogposts about LJV [in Russian](https://habr.com/ru/post/599045/) /  [in English](https://dzone.com/articles/what-can-we-learn-from-java-data-structures-visual).
>
> Ivan Ponomarev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Merge pull request #7 from atp-mipt/jchekProblems
>    
>    jcheck Whitespace error fix
>  - jcheck Whitespace error fix

All right! I have only a few minor nits from the final reading of this patch. John also had some comments, please address them as well. Then we are good to go! It would be a very important contribution to JOL, which extends its usefulness much further. Thank you all for perseverance in this PR!

Even though not required by contribution rules, I'd like to get the blessing from @robilad for accepting this contribution :)

jol-core/src/main/java/org/openjdk/jol/info/FieldLayout.java line 109:

> 107:     }
> 108: 
> 109:     public FieldData data() {

I suggest we put a little comment in method body here:


 // TODO: This is public to let LJV access it. Figure out a cleaner way to do this without extending the API surface.

jol-core/src/main/java/org/openjdk/jol/ljv/IntrospectionWithReflectionAPI.java line 90:

> 88:             if (!(Modifier.isStatic(field.getModifiers()))
> 89:                     && !ljv.canIgnoreField(field)
> 90:             ) {

Suggestion:

            if (!Modifier.isStatic(field.getModifiers()) && !ljv.canIgnoreField(field)) {

jol-core/src/main/java/org/openjdk/jol/ljv/LJV.java line 28:

> 26: 
> 27: //- Author:     John Hamer <J.Hamer at cs.auckland.ac.nz>
> 28: //- Created:    Sat May 10 15:27:48 2003

See the relevant discussion: https://github.com/openjdk/jol/pull/24/files/5001f98dc2117fbd0e0acd94ffddbccc36887acc#r993431410

jol-core/src/main/java/org/openjdk/jol/ljv/LJV.java line 240:

> 238:                         || ignoreSet.contains(field.getType())
> 239:                         || ignoreSet.contains(field.getType().getPackage())
> 240:                 ;

Suggestion:

        return Modifier.isStatic(field.getModifiers()) ||
                  ignoreSet.contains(field) ||
                  ignoreSet.contains(field.getName()) ||
                  ignoreSet.contains(field.getType()) ||
                  ignoreSet.contains(field.getType().getPackage());

jol-core/src/main/java/org/openjdk/jol/ljv/Quote.java line 41:

> 39:                 || Character.isLetter(c)
> 40:                 || Character.isDigit(c)
> 41:                 ;

Suggestion:

        return (canAppearUnquotedInLabelChars.indexOf(c) != -1) ||
                Character.isLetter(c) || Character.isDigit(c);

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

PR: https://git.openjdk.org/jol/pull/24


More information about the jol-dev mailing list