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

[Aleksey Shipilev]

On Mon, 22 Aug 2022 07:51:02 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 one additional commit since the last revision:
> 
>   update copyright notice, lint

Another round of review follows.

.github/workflows/ljv.yml line 1:

> 1: name: build

My previous question still stands: can we merge it into current workflow? It would also test things with different Java versions then.

jol-core/src/main/java/org/openjdk/jol/layouters/FieldAllocationType.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

File is not modified, we should not modify the copyright header either.

jol-core/src/main/java/org/openjdk/jol/ljv/GraphvizVisualization.java line 37:

> 35:     private final LJV ljv;
> 36:     private final IdentityHashMap<Object, String> alreadyDrawnObjectsIds = new IdentityHashMap<>();
> 37:     private boolean alreadyDrawnNull = false;

Default value, so:

Suggestion:

    private boolean alreadyDrawnNull;

jol-core/src/main/java/org/openjdk/jol/ljv/GraphvizVisualization.java line 55:

> 53:     public void diagramBegin() {
> 54:         out.setLength(0); // Clearing String Builder before starting new DOT
> 55:         out.append("digraph Java {\n")

Here and later, `\n` should actually be `System.lineSeparator()` to provide platform-independent line breaks. This would probably make golden test output platform-dependent, though? We can rehash this after the integration.

jol-core/src/main/java/org/openjdk/jol/ljv/Introspection.java line 32:

> 30: import java.util.List;
> 31: 
> 32: public interface Introspection {

Now that LJV is in JOL, can we "just" use its internals to introspect stuff, without doing LJV-specific walk? What's the plan here?

jol-core/src/main/java/org/openjdk/jol/ljv/Introspection.java line 52:

> 50:     boolean canTreatObjAsPrimitive(Object obj);
> 51: 
> 52:     boolean catTreatObjAsArrayOfPrimitives(Object obj);

Suggestion:

    boolean canTreatObjAsArrayOfPrimitives(Object obj);

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

> 52:         }
> 53: 
> 54:         // Не заци��ливаемся, смотрим обошл�� мы этот объект уже или ещё нет.

Russian comment :)

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

> 42: //-   You should have received a copy of the GNU General Public License along
> 43: //-   with this program; if not, write to the Free Software Foundation, Inc.,
> 44: //-   59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.

Wait a second, what's this? Does this file borrow some external code?

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

> 51:                 sb.append(c);
> 52:             else
> 53:                 sb.append("\\\\0u").append(Integer.toHexString(c));

Braces, please.

jol-core/src/main/java/org/openjdk/jol/ljv/pom.xml line 1:

> 1: <?xml version="1.0" encoding="UTF-8"?>

Weird to see this `pom.xml` in sources. Do you mean to merge parts of it into `jol-core/pom.xml`?

jol-core/src/main/java/org/openjdk/jol/ljv/provider/ArrayElementAttributeProvider.java line 27:

> 25: package org.openjdk.jol.ljv.provider;
> 26: 
> 27: //@FunctionalInterface

Leftover comment.

jol-core/src/main/java/org/openjdk/jol/ljv/provider/NodeType.java line 33:

> 31:     NULL_REFERENCE, //we draw it as a separate node, we do not deduplicate nulls
> 32:     //'null' value can be either PRIMITIVE, NULL or IGNORE
> 33:     IGNORE //do not show

Rewrite these comments as proper Javadoc?

jol-core/src/main/java/org/openjdk/jol/ljv/provider/ObjectAttributesProvider.java line 31:

> 29:  */
> 30: 
> 31: //@FunctionalInterface

Leftover comment.

jol-core/src/main/java/org/openjdk/jol/util/IOUtils.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

Here a later, a whole bunch of files got their copyright years updated for no reason.

jol-core/src/test/java/org/openjdk/jol/ljv/ArrayItem.java line 1:

> 1: package org.openjdk.jol.ljv;

Here and later, files are missing copyright headers.

jol-core/src/test/java/org/openjdk/jol/ljv/HashCodeCollision.java line 9:

> 7:  * @author Ilya Selivanov
> 8:  */
> 9: public class HashCodeCollision {

This file has a mix of identifier styles: `alphabet_list`, for example, should be `alphabetList`.

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

Changes requested by shade (Committer).

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