Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction #12329

uschindler · 2023-05-25T09:14:34Z

This fixes problem on branch_9x backport (#12327) where the private superclasses are not available in Java 11.

On Java 17 it just worked so we didn't figure. :-(

…action. This fixes problem on branch_9x where the private superclasses are not available in Java 11. On Java 17 it just worked so we didn't figure.

ChrisHegarty · 2023-05-25T09:17:40Z

I think that your proposed change is fine. Alternatively we could determine accessibility / visibility of the super class / interface by reading it from the jrt. But this is probably overkill for this one particular case. And of course, we'd need to consult the module-info to determine if that package is unconditionally exported - way way too much for this. ;-)

uschindler · 2023-05-25T09:33:54Z

Thanks for review, i added a logging statement to show all classes which were not referenced but not included. This would solve problems like this in future.

Will merge when CI is happy.

uschindler · 2023-05-25T09:35:00Z

Looks like this:

> Task :lucene:core:generateJdkApiJar19
Transforming 31 class files in [/modules/java.base]...
Writing 26 visible classes for [/modules/java.base]...
Writing stub for class: java/lang/foreign/AbstractLayout
Writing stub for class: java/lang/foreign/Addressable
Writing stub for class: java/lang/foreign/FunctionDescriptor
Writing stub for class: java/lang/foreign/GroupLayout
Writing stub for class: java/lang/foreign/Linker
Writing stub for class: java/lang/foreign/MemoryAddress
Writing stub for class: java/lang/foreign/MemoryLayout
Writing stub for class: java/lang/foreign/MemoryLayout$PathElement
Writing stub for class: java/lang/foreign/MemorySegment
Writing stub for class: java/lang/foreign/MemorySession
Writing stub for class: java/lang/foreign/SegmentAllocator
Writing stub for class: java/lang/foreign/SequenceLayout
Writing stub for class: java/lang/foreign/SymbolLookup
Writing stub for class: java/lang/foreign/VaList
Writing stub for class: java/lang/foreign/VaList$Builder
Writing stub for class: java/lang/foreign/ValueLayout
Writing stub for class: java/lang/foreign/ValueLayout$OfAddress
Writing stub for class: java/lang/foreign/ValueLayout$OfBoolean
Writing stub for class: java/lang/foreign/ValueLayout$OfByte
Writing stub for class: java/lang/foreign/ValueLayout$OfChar
Writing stub for class: java/lang/foreign/ValueLayout$OfDouble
Writing stub for class: java/lang/foreign/ValueLayout$OfFloat
Writing stub for class: java/lang/foreign/ValueLayout$OfInt
Writing stub for class: java/lang/foreign/ValueLayout$OfLong
Writing stub for class: java/lang/foreign/ValueLayout$OfShort
Writing stub for class: java/nio/channels/FileChannel
Referenced classes not included: [java/nio/channels/ScatteringByteChannel, java/nio/channels/GatheringByteChannel, java/nio/channels/SeekableByteChannel, java/nio/channels/spi/AbstractInterruptibleChannel, java/lang/AutoCloseable, java/lang/Object]

> Task :lucene:core:generateJdkApiJar20
Transforming 30 class files in [/modules/java.base]...
Writing 29 visible classes for [/modules/java.base]...
Writing stub for class: java/lang/foreign/Arena
Writing stub for class: java/lang/foreign/FunctionDescriptor
Writing stub for class: java/lang/foreign/GroupLayout
Writing stub for class: java/lang/foreign/Linker
Writing stub for class: java/lang/foreign/Linker$Option
Writing stub for class: java/lang/foreign/Linker$Option$CaptureCallState
Writing stub for class: java/lang/foreign/MemoryLayout
Writing stub for class: java/lang/foreign/MemoryLayout$PathElement
Writing stub for class: java/lang/foreign/MemorySegment
Writing stub for class: java/lang/foreign/PaddingLayout
Writing stub for class: java/lang/foreign/SegmentAllocator
Writing stub for class: java/lang/foreign/SegmentScope
Writing stub for class: java/lang/foreign/SequenceLayout
Writing stub for class: java/lang/foreign/StructLayout
Writing stub for class: java/lang/foreign/SymbolLookup
Writing stub for class: java/lang/foreign/UnionLayout
Writing stub for class: java/lang/foreign/VaList
Writing stub for class: java/lang/foreign/VaList$Builder
Writing stub for class: java/lang/foreign/ValueLayout
Writing stub for class: java/lang/foreign/ValueLayout$OfAddress
Writing stub for class: java/lang/foreign/ValueLayout$OfBoolean
Writing stub for class: java/lang/foreign/ValueLayout$OfByte
Writing stub for class: java/lang/foreign/ValueLayout$OfChar
Writing stub for class: java/lang/foreign/ValueLayout$OfDouble
Writing stub for class: java/lang/foreign/ValueLayout$OfFloat
Writing stub for class: java/lang/foreign/ValueLayout$OfInt
Writing stub for class: java/lang/foreign/ValueLayout$OfLong
Writing stub for class: java/lang/foreign/ValueLayout$OfShort
Writing stub for class: java/nio/channels/FileChannel
Referenced classes not included: [java/nio/channels/ScatteringByteChannel, java/nio/channels/GatheringByteChannel, java/nio/channels/SeekableByteChannel, java/nio/channels/spi/AbstractInterruptibleChannel, java/lang/AutoCloseable, java/lang/Object]
Transforming 189 class files in [/modules/jdk.incubator.vector]...
Writing 21 visible classes for [/modules/jdk.incubator.vector]...
Writing stub for class: jdk/incubator/vector/AbstractVector
Writing stub for class: jdk/incubator/vector/ByteVector
Writing stub for class: jdk/incubator/vector/DoubleVector
Writing stub for class: jdk/incubator/vector/FloatVector
Writing stub for class: jdk/incubator/vector/IntVector
Writing stub for class: jdk/incubator/vector/LongVector
Writing stub for class: jdk/incubator/vector/ShortVector
Writing stub for class: jdk/incubator/vector/Vector
Writing stub for class: jdk/incubator/vector/VectorMask
Writing stub for class: jdk/incubator/vector/VectorOperators
Writing stub for class: jdk/incubator/vector/VectorOperators$Associative
Writing stub for class: jdk/incubator/vector/VectorOperators$Binary
Writing stub for class: jdk/incubator/vector/VectorOperators$Comparison
Writing stub for class: jdk/incubator/vector/VectorOperators$Conversion
Writing stub for class: jdk/incubator/vector/VectorOperators$Operator
Writing stub for class: jdk/incubator/vector/VectorOperators$Ternary
Writing stub for class: jdk/incubator/vector/VectorOperators$Test
Writing stub for class: jdk/incubator/vector/VectorOperators$Unary
Writing stub for class: jdk/incubator/vector/VectorShape
Writing stub for class: jdk/incubator/vector/VectorShuffle
Writing stub for class: jdk/incubator/vector/VectorSpecies
Referenced classes not included: [java/lang/Enum, java/lang/Object]

ChrisHegarty · 2023-05-25T09:37:04Z

I'm clearly doing something wrong, but I cherrypicked this into the 9x_panama_vector branch locally, and it still fails. I dunno why, it should work. Checking that my env is correct and that I've not missed something.

I still see extends jdk.internal.vm.vector.VectorSupport$Vector<E> in Vector.class

uschindler · 2023-05-25T09:41:17Z

Did you generate everything?

uschindler · 2023-05-25T09:41:42Z

Ahhhhh, the signature because it is generic....

uschindler · 2023-05-25T09:46:37Z

Here you see it, must be the signature as it shows generics:

>javap Vector.class | grep internal
public abstract class jdk.incubator.vector.Vector<E> extends jdk.internal.vm.vector.VectorSupport$Vector<E> {

Uwe

ChrisHegarty · 2023-05-25T09:49:48Z

Here's what I'm doing:

$ ./gradlew :lucene:core:regenerate

Debugging the extractor Cleaner, I can see that superName is replaced with java/lang/Object.

$ cat T.java
import jdk.incubator.vector.*;

public class T {
    static final FloatVector acc1 = FloatVector.zero(FloatVector.SPECIES_PREFERRED);
}
$ 
$ /Users/chegar/binaries/jdk-11.0.12.jdk/Contents/Home/bin/javac --patch-module java.base=lucene/core/src/generated/jdk/jdk20.apijar --add-exports=java.base/jdk.incubator.vector=ALL-UNNAMED T.java
T.java:4: error: cannot access VectorSupport
    static final FloatVector acc1 = FloatVector.zero(FloatVector.SPECIES_PREFERRED);
                                               ^
  class file for jdk.internal.vm.vector.VectorSupport not found


cd lucene/core/src/generated/jdk
$ unzip jdk20.apijar 
...
javap -v -p jdk/incubator/vector/Vector.class  | more
Classfile /Users/chegar/git/lucene/lucene/core/src/generated/jdk/jdk/incubator/vector/Vector.class
  Last modified 1 Jan 2022; size 10382 bytes
  MD5 checksum 9d8f0c7cc22956e602994bfc4f1507b4
public abstract class jdk.incubator.vector.Vector<E extends java.lang.Object> extends jdk.internal.vm.vector.VectorSupport$Vector<E>
...

uschindler · 2023-05-25T10:00:46Z

The problem is the signature, I verified it. Let me think how to do that.

uschindler · 2023-05-25T10:31:30Z

This can't be repaired, as the generic signature refers to the other class which has generics, too. By replacing it with java.lang.Object it breaks horribly (as Object has no type parameters).

If I just null out the generic signature, javac reports:

  bad class file: C:\Users\Uwe Schindler\Projects\lucene\lucene\lucene\core\src\generated\jdk\jdk20.apijar(/jdk/incubator/vector/Vector.class)
    undeclared type variable: E

So the only route here is my original idea to include the internal classes intothe JAR -- unfortunately.

uschindler · 2023-05-25T10:32:31Z

Keep the backport open, I will try to fix it later this afternoon, I first have to do something else.

ChrisHegarty · 2023-05-25T11:09:14Z

For now, I just updated the glob in the 9x Panama vector branch to include VectorSupport, see 7c353f0

--- a/gradle/generation/extract-jdk-apis/ExtractJdkApis.java
+++ b/gradle/generation/extract-jdk-apis/ExtractJdkApis.java
@@ -50,7 +50,7 @@ public final class ExtractJdkApis {
   private static final FileTime FIXED_FILEDATE = FileTime.from(Instant.parse("2022-01-01T00:00:00Z"));
   
   static final Map<String,String> CLASSFILE_MATCHERS = Map.of(
-      "java.base",             "glob:java/{lang/foreign/*,nio/channels/FileChannel,util/Objects}.class",
+      "java.base",             "glob:{java/lang/foreign/*,java/nio/channels/FileChannel,java/util/Objects,jdk/internal/vm/vector/VectorSupport*}.class",
       "jdk.incubator.vector",  "glob:jdk/incubator/vector/*.class"
   );

…ted as "hidden" and we remove all their members

ChrisHegarty · 2023-05-25T12:50:08Z

Argh! inner classes!!! We still need the enclosing class (which we currently don't have).

the jdk20api.jar contains:

  inflating: jdk/internal/vm/vector/VectorSupport$Vector.class  
  inflating: jdk/internal/vm/vector/VectorSupport$VectorMask.class  
  inflating: jdk/internal/vm/vector/VectorSupport$VectorPayload.class  
  inflating: jdk/internal/vm/vector/VectorSupport$VectorShuffle.class

uschindler · 2023-05-25T13:01:19Z

I have seen this, too, but does this break in the Java 11 compiler?

ChrisHegarty · 2023-05-25T13:09:04Z

yeah, it breaks - in the same way - "class file for jdk.internal.vm.vector.VectorSupport not found"

uschindler · 2023-05-25T13:31:53Z

I have no idea way they had did this crazy class layout. There's also the special javadoc tag "@hidden" which looks like a crazy workaround.

It looks like we have to include also the superclasss. There are also interfaces which on the other hand are not referenced downstream, so the current code to hide them is fine.

uschindler · 2023-05-25T13:32:27Z

Have a look at the whole class: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java

uschindler · 2023-05-25T13:53:43Z

Here is this mess with the @hidden Javadoc tag explained: https://bugs.openjdk.org/browse/JDK-8277074

As you see we are in the areas of heavy hacks with visibility. So we can't solve this in a general way. I tend to hardcode the exact classes needed into the extractor as patterns.

I will also change the patterns a bit by having a list of filename patterns without module per Java version.

ChrisHegarty · 2023-05-25T14:03:30Z

So we can't solve this in a general way. I tend to hardcode the exact classes needed into the extractor as patterns.

++ while not perfect, it is understandable and maintainable. The extra logging is also very welcome, which will help future potential changes in this area.

- have a list of filematcher patterns per java version - collect all files using a combined matcher

uschindler · 2023-05-25T15:01:35Z

OK, I committed my final fix. Rewrote the modules / java version static settings at top of file:

For each Java version it has a list of glob patterns
To simplify the common patterns are string constants (3, panama-foreign, vector, vm internals)
The patterns are combined and the filesystem is walked only one time

The internal classes are explicitely specifid until no more were left over in the warning at end (including the superclass).

uschindler · 2023-05-25T15:06:27Z

@ChrisHegarty: do you want to crosscheck the 9.x branch? (don't forget to re-add the java/util/Objects class pattern).

ChrisHegarty

The changes look good. And maintainable.

I can confirm that it works on the 9x Panama_vector branch. 👍

uschindler · 2023-05-25T15:22:31Z

Thanks, as always a fun to work together!

ChrisHegarty · 2023-05-25T15:25:42Z

Thanks, as always a fun to work together!

Absolutely. And thanks for fixing this so quickly. So how to proceed? I can merge in this PR commit (to he 9x_panama_branch), or we can coordinate and you can backport very quickly afterwards?

uschindler · 2023-05-25T15:29:54Z

I would merge this to the 9x_panama_branch. I don't want the huge binary file go into the main repo's Git state (I am afraid of Dawid and Robert about binaries in checkouts).

uschindler · 2023-05-25T15:30:30Z

The Java 21 branch was also updated.

…ces during extraction (apache#12329)

uschindler · 2023-05-25T16:08:03Z

Here is this mess with the @hidden Javadoc tag explained: https://bugs.openjdk.org/browse/JDK-8277074

As you see we are in the areas of heavy hacks with visibility. So we can't solve this in a general way. I tend to hardcode the exact classes needed into the extractor as patterns.

I will also change the patterns a bit by having a list of filename patterns without module per Java version.

By the way in Java 19 you see the @hidden Javadoc classes also in the public Javadocs. So it is still a mess (now behind the scenes): https://docs.oracle.com/en/java/javase/19/docs/api//jdk.incubator.vector/jdk/incubator/vector/Vector.html

Remove jdk.internal classes from superclass or interfaces during extr…

092b28d

…action. This fixes problem on branch_9x where the private superclasses are not available in Java 11. On Java 17 it just worked so we didn't figure.

uschindler added the type:bug label May 25, 2023

uschindler added this to the 9.7.0 milestone May 25, 2023

uschindler requested a review from ChrisHegarty May 25, 2023 09:14

uschindler self-assigned this May 25, 2023

uschindler mentioned this pull request May 25, 2023

[Backport] Integrate the Incubating Panama Vector API #12311 #12327

Merged

ChrisHegarty approved these changes May 25, 2023

View reviewed changes

add logging of not included classes (to ease debugging)

f8e2d03

uschindler marked this pull request as draft May 25, 2023 11:07

2nd approach, now we extract all files. The internal classes are trea…

e4ddb9f

…ted as "hidden" and we remove all their members

uschindler changed the title ~~Remove jdk.internal classes from superclass or interfaces during extraction~~ Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction May 25, 2023

Rewrite the logic:

687a9eb

- have a list of filematcher patterns per java version - collect all files using a combined matcher

uschindler marked this pull request as ready for review May 25, 2023 15:11

uschindler requested a review from ChrisHegarty May 25, 2023 15:15

ChrisHegarty approved these changes May 25, 2023

View reviewed changes

uschindler merged commit c188d47 into apache:main May 25, 2023

uschindler deleted the dev/extractor_remove_jdk_internal branch May 25, 2023 15:21

ChrisHegarty pushed a commit to ChrisHegarty/lucene that referenced this pull request May 25, 2023

Handle jdk.internal classes mentioned in vector superclass or interfa…

e117f28

…ces during extraction (apache#12329)

uschindler mentioned this pull request May 26, 2023

Integrate the Incubating Panama Vector API #12311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction #12329

Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction #12329

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

ChrisHegarty commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023 •

edited

Loading

ChrisHegarty left a comment

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023 •

edited

Loading

Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction #12329

Handle jdk.internal classes mentioned in vector superclass or interfaces during extraction #12329

Conversation

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 • edited Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 • edited Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023 • edited Loading

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

ChrisHegarty commented May 25, 2023 • edited Loading

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023 • edited Loading

uschindler commented May 25, 2023 • edited Loading

ChrisHegarty left a comment

Choose a reason for hiding this comment

uschindler commented May 25, 2023

ChrisHegarty commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023

uschindler commented May 25, 2023 • edited Loading

ChrisHegarty commented May 25, 2023 •

edited

Loading

ChrisHegarty commented May 25, 2023 •

edited

Loading

ChrisHegarty commented May 25, 2023 •

edited

Loading

ChrisHegarty commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023 •

edited

Loading

uschindler commented May 25, 2023 •

edited

Loading