googleapis / sdk-platform-java

Tooling and shared libraries for Cloud SDK for Java
https://cloud.google.com/java/docs/bom
Apache License 2.0
65 stars 54 forks source link

GraalVM: Error with RunReachabilityHandlersConcurrently build argument #1344

Closed ddobrin closed 1 year ago

ddobrin commented 1 year ago

In GraalVM, while upgrading to 22.2 the team had to disable concurrent reachability since it was causing some failures in the client libraries when building native images: Issue.

In GraalVM, latest versions (22.3.0 and 22.3.1), disabling the RunReachabilityHandlersConcurrently build argument introduces the need to add a number of additional dependencies (rome tools and kotlinx). GraalVM has it turned on by default.

The issue can be alleviated by reenabling the build argument in an application. Reproducer: https://github.com/ddobrin/serverless-photosharing-workshop/tree/master/services/image-analysis/java

Tested with the latest BOM version as of today: 26.7.0.

            <plugin>
                <groupId>org.graalvm.buildtools</groupId>
                <artifactId>native-maven-plugin</artifactId>
                <configuration>
                    <buildArgs>
                        <arg>-H:+RunReachabilityHandlersConcurrently</arg>
                    </buildArgs>                
                </configuration>
            </plugin>  

The ask: Could the issue be re-validated in the client libraries with GraalVM 22.3.x to ascertain whether the flag is still creating an issue?

Thank you

mpeddada1 commented 1 year ago

Thank you for your investigation @ddobrin! Sharing some findings here after testing this out. Unfortunately, it appears that the issue persists when RunReachabilityHandlersConcurrently is enabled despite upgrading to 22.3.0 and 22.3.1 (https://github.com/mpeddada1/graalvm22.2-reachability#does-this-sample-work-without-the-workaround-in-graalvm-2231-latest)

===================================================================================
GraalVM Native Image: Generating 'native-tests' (executable)...
===================================================================================
[1/7] Initializing...                                               (5.1s @ 0.22GB)
 Version info: 'GraalVM 22.3.1 Java 11 CE'
 Java version info: '11.0.18+10-jvmci-22.3-b13'
 C compiler: gcc (linux, x86_64, 12.2.0)
 Garbage collector: Serial GC
 2 user-specific feature(s)
 - com.example.MyNativeImageFeature
 - org.graalvm.junit.platform.JUnitPlatformFeature
[junit-platform-native] Running in 'test listener' mode using files matching pattern [junit-platform-unique-ids*] found in folder [${HOME}/graalvm22.2-reachability/child-module/target/test-ids] and its subfolders.
[2/7] Performing analysis...  [******]                             (11.0s @ 1.58GB)
   4,363 (79.10%) of  5,516 classes reachable
   5,430 (58.46%) of  9,288 fields reachable
  19,097 (50.47%) of 37,835 methods reachable
     155 classes,     2 fields, and   545 methods registered for reflection
      58 classes,    58 fields, and    52 methods registered for JNI access
       4 native libraries: dl, pthread, rt, z
[3/7] Building universe...                                          (1.9s @ 0.85GB)
[4/7] Parsing methods...      [*]                                   (1.4s @ 1.92GB)
[5/7] Inlining methods...     [***]                                 (0.9s @ 2.53GB)
[6/7] Compiling methods...    [***]                                 (8.4s @ 1.52GB)
[7/7] Creating image...                                             (1.8s @ 2.11GB)
   6.23MB (37.56%) for code area:    11,308 compilation units
   9.52MB (57.38%) for image heap:  120,459 objects and 7 resources
 859.91KB ( 5.06%) for other data
  16.59MB in total
-----------------------------------------------------------------------------------
Top 10 packages in code area:            Top 10 object types in image heap:
 816.60KB java.util                         1.35MB byte[] for code metadata
 394.53KB com.sun.crypto.provider           1.11MB java.lang.String
 358.69KB java.lang                         1.02MB java.lang.Class
 284.24KB java.util.concurrent            977.52KB byte[] for general heap data
 224.78KB java.text                       824.72KB byte[] for java.lang.String
 207.10KB java.util.stream                410.53KB java.util.HashMap$Node
 205.18KB java.util.regex                 409.03KB c.o.s.c.h.DynamicHubCompanion
 183.88KB java.io                         231.64KB java.util.HashMap$Node[]
 162.89KB sun.security.provider           205.72KB java.lang.String[]
 154.35KB java.math                       195.61KB j.u.c.ConcurrentHashMap$Node
   3.22MB for 190 more packages             1.84MB for 1076 more object types
-----------------------------------------------------------------------------------
      1.1s (3.3% of total time) in 19 GCs | Peak RSS: 4.89GB | CPU load: 6.93
-----------------------------------------------------------------------------------
Produced artifacts:
${HOME}/graalvm22.2-reachability/child-module/target/native-tests (executable)
${HOME}/graalvm22.2-reachability/child-module/target/native-tests.build_artifacts.txt (txt)
===================================================================================
Finished generating 'native-tests' in 32.2s.
[INFO] Executing: ${HOME}/graalvm22.2-reachability/child-module/target/native-tests --xml-output-dir ${HOME}/graalvm22.2-reachability/child-module/target/native-test-reports -Djunit.platform.listeners.uid.tracking.output.dir=${HOME}/graalvm22.2-reachability/child-module/target/test-ids
JUnit Platform on Native Image - report
----------------------------------------

**********VALUE of USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM************
null
com.example.MySampleTest > testSample FAILED

Failures (1):
  JUnit Vintage:MySampleTest:testSample
    MethodSource [className = 'com.example.MySampleTest', methodName = 'testSample', methodParameterTypes = '']
    => java.lang.RuntimeException: Generated message class "com.example.MySampleClass" missing method "getName".
       com.anotherpackage.GeneratedMessage.retrieveMethod(GeneratedMessage.java:31)
       com.anotherpackage.GeneratedMessage.access$000(GeneratedMessage.java:5)
       com.anotherpackage.GeneratedMessage$MethodAccessor.<init>(GeneratedMessage.java:20)
       com.anotherpackage.GeneratedMessage$A.initializeMethodAccessor(GeneratedMessage.java:11)
       com.example.MySampleClass.invokeAccessor(MySampleClass.java:13)
       [...]
     Caused by: java.lang.NoSuchMethodException: com.example.MySampleClass.getName()
       java.base@11.0.18/java.lang.Class.getMethod(DynamicHub.java:2108)
       com.anotherpackage.GeneratedMessage.retrieveMethod(GeneratedMessage.java:28)
       [...]

I'm also able to reproduce the same issue when running tests in java-datatore (mvn test -Dtest=com.google.cloud.datastore.admin.v1.DatastoreAdminClientTest -Pnative -DfailIfNoTests=false) locally with -H:+RunReachabilityHandlersConcurrently

mpeddada1 commented 1 year ago

From conversation in https://github.com/googleapis/gax-java/pull/1815#issuecomment-1432812163: The GraalVM team is tracking this issue(https://github.com/oracle/graal/issues/5194) and will follow up on their findings. Will keep an eye on it in the meantime.

ddobrin commented 1 year ago

Thanks for the update. We'll see when the fix for https://github.com/oracle/graal/issues/5868 is released.

mpeddada1 commented 1 year ago

The fix for the original issue was merged and added as a milestone for the upcoming feature release - GraalVM for JDK 17 / JDK 20

zakkak commented 1 year ago

FYI:

GraalVM for JDK 17 / JDK 20 is now released and includes the said fix.

I have also opened a backport PR to get the fix in 22.3.3 as well .

mpeddada1 commented 1 year ago

Thanks so much for the heads up and putting up the PR @zakkak!

zakkak commented 1 year ago

GraalVM 22.3.3 is also released now, rendering -H:-RunReachabilityHandlersConcurrently unnecessary for GraalVM >= 22.3.3

@mpeddada1 is there anything preventing it's removal? What are the GraalVM versions that sdk-platform-java supports?

mpeddada1 commented 1 year ago

Hi @zakkak, we've merged the PR that removes this argument: https://github.com/googleapis/sdk-platform-java/pull/1892. It will be included in release 2.25.0 of this repo.

zakkak commented 1 year ago

Great, thanks for the update @mpeddada1!

mpeddada1 commented 1 year ago

@zakkak @ddobrin Thanks for your patience. This change is now part of libraries-bom:26.23.0!