search

datastax / cassandra-quarkus

An Apache Cassandra(R) extension for Quarkus
Apache License 2.0
40 stars 28 forks source link

JAVA-2782: Add substitutions to make Guava more Graal-friendly #14

Closed absurdfarce closed 4 years ago

absurdfarce commented 4 years ago

I went back and forth on whether the substitution class should have some comments spelling out the problem. Philosophies always vary here but it seemed like terseness was more consistent with the rest of the code base. The problem is already stated fairly clearly on the JIRA ticket so maybe a pointer to that wouldn't be the worst thing.

absurdfarce commented 4 years ago

After adding this substitution I started seeing some odd errors indicating that Graal's analysis was finding paths to ClassLoader.defineClass() in the jnr-posix code. This was... odd since this very behaviour was addressed in JAVA-2663. Then I remembered that the Java driver version in use here is 4.6.1; 2663 went in after that was released. So when we update the Java driver version this will disappear. I confirmed this expectation by testing using a 4.7.0-SNAPSHOT driver which contained 2663... as expected no warnings were displayed at all.

absurdfarce commented 4 years ago

My output locally:

[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] /work/local/graalvm-ce-java8-19.3.1/jre/bin/native-image -J-Dsun.nio.ch.maxUpdateArraySize=100 -J-Djava.util.logging
.manager=org.jboss.logmanager.LogManager -J-Dvertx.logger-delegate-factory-class-name=io.quarkus.vertx.core.runtime.VertxLogDelegateFactory -J-Dvertx.disableDnsResolver=true -J-D
io.netty.leakDetection.level=DISABLED -J-Dio.netty.allocator.maxOrder=1 -J-Duser.language=en -J-Dfile.encoding=UTF-8 --initialize-at-build-time= -H:InitialCollectionPolicy=com.or
acle.svm.core.genscavenge.CollectionPolicy$BySpaceAndTime -H:+JNI -jar cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner.jar -H:FallbackThreshold=0 -H:+ReportExceptionStackTrace
s -H:-AddAllCharsets -H:-IncludeAllTimeZones -H:EnableURLProtocols=http -H:NativeLinkerOption=-no-pie --no-server -H:-UseServiceLoaderFeature -H:+StackTrace cassandra-quarkus-qui
ckstart-1.0.0-SNAPSHOT-runner                                                                                                                                                     
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]    classlist:  22,017.68 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]        (cap):   1,375.96 ms                  
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]        setup:   4,013.92 ms                                                                                            
00:02:59,535 INFO  [com.dat.oss.dri.int.cor.DefaultMavenCoordinates] DataStax Java driver for Apache Cassandra(R) (com.datastax.oss:java-driver-core) version 4.7.0-SNAPSHOT
00:03:01,790 INFO  [org.jbo.threads] JBoss Threads version 3.0.1.Final                                                                                                            
00:03:36,847 INFO  [com.dat.oss.dri.int.cor.os.Native] Using Graal-specific native functions                                                                                     
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]   (typeflow):  82,497.52 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]    (objects):  30,025.19 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]   (features):   1,016.94 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]     analysis: 116,896.98 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]     (clinit):   1,679.41 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]     universe:   4,793.83 ms            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]      (parse):  11,510.92 ms                                                                          
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]     (inline):  12,972.32 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]    (compile):  66,928.78 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]      compile:  94,489.19 ms                                                
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]        image:   4,743.04 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]        write:     788.25 ms                                                                                            
[cassandra-quarkus-quickstart-1.0.0-SNAPSHOT-runner:16683]      [total]: 248,373.21 ms                                                                                            
[INFO] [io.quarkus.deployment.QuarkusAugmentor] Quarkus augmentation completed in 250710ms
absurdfarce commented 4 years ago

Huh, JNR errors also now appearing on Jenkins... see https://jenkins-drivers.build.dsinternal.org/blue/organizations/jenkins/tools%2Fcassandra-quarkus/detail/java2782/1/pipeline/8 as an example.

Not sure why this would suddenly appear given the addition of a new substitution. I'll dig into this a bit more, see if there's a simple way to fix this now knowing that the "right" fix is coming with JAVA-2663.

absurdfarce commented 4 years ago

Native integration tests run cleanly for me locally, assuming that last failure was something environmental... so let's try this again

absurdfarce commented 4 years ago

I'm gonna close this PR in favor of distinct PRs for the two issues at play here. These issues are logically distinct and can be resolved at different times (the native stuff should be largely ready to go now) so there's no real reason to continue to tie them together.

New PR for native substitution is https://github.com/datastax/cassandra-quarkus/pull/16

New PR for Guava fix is https://github.com/datastax/cassandra-quarkus/pull/17