RFR: Extend API to support JOL IntelliJ Idea plugin

[Aleksey Shipilev]

On Wed, 30 Sep 2020 13:31:03 GMT, Sergey Ponomarev <github.com+415502+stokito at openjdk.org> wrote:

> [JOL Intellij Idea Plugin](https://github.com/stokito/IdeaJol)  internally uses JOL.
> Internally in the
> [PsiClassAdapter](https://github.com/stokito/IdeaJol/blob/master/src/main/java/com/github/stokito/IdeaJol/PsiClassAdapter.java)
> it makes exactly the same as `org.openjdk.jol.info.ClassData#parse` but it creates the ClassData from Idea's AST/Psi
> tree instead of raw `Class`. Unfortunately to be able to do this we need to extend API a little bit.  Here is a short
> list of changes that I did: 1. Added an overloaded method ClassData.addSuperClassData(ClassData) so I can create an
> instance of `ClassData`  2. Added `lossesInternal`, `lossesExternal` and `lossesTotal` fields to `ClassLayout` so they
> calculated only once on instance creation and then can be accessed multiple times via geters by JTable  3.
> `FieldData.create()` internally evaluate if the field is contended but we do this based on Psi tree. So we need to open
> and make public the `FieldData` constructor.

Looks sensible. Please rename the PR to e.g. "Extend API to support JOL IntelliJ Idea plugin"?

This looks fine. Again, I'll integrate once OCA and other integration blockers resolve and bots allow me to integrate.

jol-core/src/main/java/org/openjdk/jol/info/ClassLayout.java line 130:

> 128:      * @param check        whether to check important invariants
> 129:      */
> 130:     public static ClassLayout createClassLayout(ClassData classData, SortedSet<FieldLayout> fields, int
> headerSize, long instanceSize, boolean check) {

`ClassLayout.createClassLayout` seems tautological, maybe `ClassLayout.create`?

jol-core/src/main/java/org/openjdk/jol/info/ClassLayout.java line 145:

> 143:         }
> 144:         long lossesExternal = instanceSize != nextFree ? instanceSize - nextFree : 0;
> 145:         long lossesTotal = lossesInternal + lossesExternal;

I think this whole block would be more readable if we shorten the variable names.

        long next = headerSize;
        long internal = 0;
        for (FieldLayout fl : fields) {
            if (fl.offset() > nextFree) {
                internal += fl.offset() - nextFree;
            }
            next = fl.offset() + fl.size();
        }
        long external = (instanceSize != next) ? (instanceSize - next) : 0;
        long total = internal + external;

jol-core/src/test/java/org/openjdk/jol/info/ClassDataTest.java line 93:

> 91:     assertTrue(classFields.isEmpty());
> 92:   }
> 93: }

Add a newline at the end of file.

jol-core/src/main/java/org/openjdk/jol/info/FieldData.java line 77:

> 75:     private final String contendedGroup;
> 76:
> 77:     public FieldData(Field refField, String hostKlass, String fieldName, String fieldType,

Don't you want to add the overloaded `FieldData.create` instead?

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

Changes requested by shade (Committer).

PR: https://git.openjdk.java.net/jol/pull/4Marked as reviewed by shade (Committer).