Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ByteBuddy is incompatible with CDS due to the Java 6 baseline #1657

Open
sdeleuze opened this issue Jun 17, 2024 · 21 comments
Open

ByteBuddy is incompatible with CDS due to the Java 6 baseline #1657

sdeleuze opened this issue Jun 17, 2024 · 21 comments
Assignees
Milestone

Comments

@sdeleuze
Copy link

When running an application and Spring Boot 3.3 CDS support and Bytebuddy (typically Spring Data JPA applications like Petclinic), a lot of warnings are visible in the logs like [warning][cds] Skipping net/bytebuddy/description/type/TypeDefinition: Old class has been linked. I think this is due to the Java 6 baseline used by ByteBuddy, a Java 8 baseline would make ByteBuddy CDS compliant.

@raphw
Copy link
Owner

raphw commented Jun 17, 2024

Byte Buddy is used in a lot of tooling which normally needs to support older VMs, too. Not sure why the class data sharing excludes old class files, but for now I think the benefits of an old class file format out weight the negatives.

@raphw raphw self-assigned this Jun 17, 2024
@raphw raphw added the question label Jun 17, 2024
@raphw raphw modified the milestones: 0.4.1, 1.14.16 Jun 17, 2024
@odrotbohm
Copy link
Contributor

Would a multi-release JAR be a path out of this?

@raphw
Copy link
Owner

raphw commented Jun 17, 2024

That would work, would however double the jar size. Possibly I could offer two artifacts. I will look into that.

Does class data sharing even support MR jars?

@sdeleuze
Copy link
Author

sdeleuze commented Jul 3, 2024

Yes, I got the confirmation from the Java Platform team that MR jars would solve this. See also the related issue https://bugs.openjdk.org/browse/JDK-8288334.

@raphw
Copy link
Owner

raphw commented Aug 29, 2024

As I understand it: The JDK might support older class files at some point?

@sdeleuze
Copy link
Author

sdeleuze commented Sep 13, 2024

Short term no, in Java 25 potentially (see https://bugs.openjdk.org/browse/JDK-8317269) but I still think publishing a JDK8 version of Bytebuddy is a good idea as it brings other benefits to all users by making the class verification faster when CDS is not in use (that's a guidance that comes from directly from the Java platform team) and that would avoid requiring the latest JDK so those improvements would reach a much larger audience.

BTW this is what Netty plan to do short term, they were until now using old bytecode in Netty 4.1 and they are going to switch to JDK8 (without using MR jars) very soon in Netty 4.2.

@raphw
Copy link
Owner

raphw commented Sep 13, 2024

I agree, generally speaking I would like to offer Byte Buddy as 1.8 class files. But I would want to allow adding a classifier or some other dependency configuration option, to allow tool vendor (typically for Java agents), to switch back to another artifact.

I will investigate how this could work and try to offer a solution. And please offer a solution if you already have an idea, I have to read up on this myself, too.

@raphw
Copy link
Owner

raphw commented Sep 14, 2024

I made an attempt now with byte-buddy-agent and byte-buddy-dep. I compile the sources twice and create additional jars with the classifier java8. Would that work? Could you build Byte Buddy from the multiple-jars branch with the java8Artifact profile active, and try (with byte-buddy-agent or byte-buddy-dep)? This should work in theory. This could be activated in the dependency configurations, for example, and solve the problem for those in need.

I have not yet made up my mind if I might invert this, but maybe I will start this way at least, especially since the problem might be temporary. Unfortunately, there is still quite some Java 5/6 in banks and insurances, so I am not sure when I give up on supporting such old JVMs.

@raphw
Copy link
Owner

raphw commented Sep 14, 2024

On a side note: ASM will always be compiled to Java 5. Will that be a problem?

@sdeleuze
Copy link
Author

sdeleuze commented Oct 15, 2024

Using the java8 classifier artifact allows to go from 879 to 46 CDS warnings on Spring Petclinic so I guess that's worth even if a few ASM classes are indeed still mentioned.

The question now is to see if Hibernate is willing to update their build to use this potential this java8 classifier artifact, since we would like to avoid asking users to do it manually. Is it something we can check with them or should we?

@raphw
Copy link
Owner

raphw commented Oct 15, 2024

Possibly, I can merge this in as a MR jar. This would blow up the artifact size, but I wonder if this really matters in today's world. Would make it an implicit default, too, what I'd think is preferable.

@sdeleuze
Copy link
Author

Happy to test a MR jar locally if you add that to a brancg, I agree that getting the implicit default would be much better.

@raphw
Copy link
Owner

raphw commented Oct 15, 2024

Try the mr-java8 branch. The JAR is 8 MB now, that is somewhat a concern, but I would not think that is an actual problem but rather a cosmetic one.

@sdeleuze
Copy link
Author

Works as expected, I just had to override the version of the net.bytebuddy:byte-buddy dependency with the one locally built, and I just had the 46 CDS warnings instead of the 879 ones.

@raphw
Copy link
Owner

raphw commented Oct 16, 2024

I think I will implement this then. Normally jar size is not important as dependencies are cached. For agents, the mr feature can be unrolled as one can strip one of the versions out. Byte Buddy is already large as it covers the entire Java language and its instrumentation features, so the few people who have an issue with this will likely be able to address it.

@raphw
Copy link
Owner

raphw commented Oct 21, 2024

Just FYI: I worked out a small Maven plugin that takes the shaded ASM dependency, computes stack map frames and stores a Java 8 version of ASM in the multi-release jar, alongside the Byte Buddy classes. This should take care of the remaining 46 CDS warnings for you.

@raphw
Copy link
Owner

raphw commented Oct 21, 2024

Still edging out some things, but this will not take too long to figure out.

@raphw raphw reopened this Oct 21, 2024
@raphw
Copy link
Owner

raphw commented Oct 21, 2024

@sdeleuze Could you give this another try and:

Build https://github.com/raphw/bytecode-update-maven-plugin
Then build this branch: https://github.com/raphw/byte-buddy/tree/asm-multi-release

After that, CDS should work with byte-buddy.jar without any warnings.

@sdeleuze
Copy link
Author

That would be super cool to include a shaded CDS friendly ASM dependency.

I try what you suggested, and updated my Petclinic application to use the locally build net.bytebuddy:byte-buddy:1.15.6-SNAPSHOT dependency, but I still see the same warnings, for example:

[4.292s][warning][cds] Skipping net/bytebuddy/jar/asm/MethodVisitor: Old class has been linked
[4.292s][warning][cds] Skipping net/bytebuddy/jar/asm/AnnotationWriter: Old class has been linked

To double check, I then checked the bytecode version of an ASM class:

asm$ javap -v MethodVisitor.class | grep major
  major version: 49

Which means Java 5.

Could you please double check if everything works as expected on your side and if I should modify something in the repos to get the ASM classes compiled with Java 8.

@raphw
Copy link
Owner

raphw commented Oct 21, 2024

Sorry, add the -Pmulti-release profile when building Byte Buddy.

PS: can you link me to how I can check this myself?

@sdeleuze
Copy link
Author

sdeleuze commented Oct 21, 2024

Ah, all good now, thanks!

In order to reproduce:

git clone https://github.com/spring-projects/spring-petclinic
cd spring-petclinic
nano pom.xml # Here add a dependency to net.bytebuddy:byte-buddy:1.15.6-SNAPSHOT to override the version
mvn clean package -DskipTests
java -Djarmode=tools -jar target/spring-petclinic-3.3.0-SNAPSHOT.jar extract --destination target/application
java -XX:ArchiveClassesAtExit=application.jsa -Dspring.context.exit=onRefresh -jar target/application/spring-petclinic-3.3.0-SNAPSHOT.jar
java -XX:SharedArchiveFile=application.jsa -jar target/application/spring-petclinic-3.3.0-SNAPSHOT.jar

The remaining warnings for proxies are expected and harmless.

See https://docs.spring.io/spring-boot/reference/packaging/efficient.html#packaging.efficient.unpacking and https://docs.spring.io/spring-boot/reference/packaging/class-data-sharing.html related documentations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants