-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Increased compilation times with GraalVM 21.1.0 #18360
Comments
Can you try to build 2.0.0.Final with GraalVM 21.0? It should work and it would help to understand if Quarkus is the issue or GraalVM. |
|
Can you try to enable |
/cc @zakkak |
This is a known GraalVM issue see oracle/graal#3435 and oracle/graal#3427 The increased compilation times appear to be linked to increased heap usage (as reported in oracle/graal#3427), since we have not yet identified the cause of the increased memory usage if you have the resources you may try to increase the max heap size to reduce the GC cycles. |
That could explain the regression with GraalVM 21.1 (but, gosh, it's drastic...) but that wouldn't explain the regression we have with both Quarkus versions using 21.0. We need to figure that out. |
I have not been able to configure
I tried with I have deactivated the report, and kept a |
Can you try doing it locally instead of doing it on OpenShift? |
Bonjour @vsevel! As @gsmet said, it would help to run this locally somewhere with enough heap (16GB or more) so that native image reports successfully complete. If this is not successful, we can build some byteman rules that could enable to get a better understanding on what's causing the differences. Could you give us a list of quarkus extensions that you're using? |
hello @galderz yes definitely, I am still planing to do this. it has been taken more time than I wanted. I will let you know. |
hello, I have managed to get some results. I have created a host with
I have the detailed log for each run (if needed), plus a the GraalVM reports for the |
detail for all runs:
|
here is the case where the reports have been attached: https://access.redhat.com/support/cases/#/case/02987371 |
I am guessing that this is something to do with reflection registration for the kube API. |
Hello @stuartwdouglas I have attached the following files to the case:
|
Between 21.1 and 21.0 the main difference seems to be a lot of these (almost 700 extra):
|
Between 1.13 and 2.0 it appears way more of the kube API's get registered for reflection. |
do you think this is going to increase with new apis getting supported? |
@vsevel I've been looking at the discrepancies between GraalVM 21.1 and 21.0 and in particular those MethodHandle reflection registrations that @stuartwdouglas talked about. Looking at the call tree, it would seem to me that your code is using MethodHandles directly, can you confirm? If that is the case, could you find a way to not use MethodHandles nor Reflection? We have plenty of experience in Quarkus in moving away from those, since they have a considerable cost. @stuartwdouglas and others can explain strategies that Quarkus uses to avoid the need for these, depending on the use case. Do MethodHandles functionality work as expected in both 21.1 and 21.0 for your use case? I'm also trying to see what's exactly made them more expensive between these two versions. |
hi @galderz, that is not the case. in the smbj extension I wrote, I had to enable reflection for: other than that we are using quarkus extensions only. we are using extensively fabric8 to deploy our openshift components to the cluster. I am not sure anything matches your definition of using |
The extension that are being added to the kubernetes-client are not included in the kubernetes-client extension. So, I don't see this been a problem. |
With a quick glimpse I only see some CustomResource related stuff. Is there something additional? |
I have started commenting out code in the application, and ran multiple native builds on Quarkus 2.0 and GraalVM 21.1. I executed 2 builds for each code base (still I see some inconsistencies in some of the results; although the vm is the same and nothing else is running on it - I am going to another pass). without toString means I commented out the It seems there is some impact on the
|
@vsevel Indeed your code does not use MethodHandles (sorry for the confusion). I think what is happening here is that you have some long String |
FYI: the JEP that brought the improvements to String concatenation. |
here is the 2nd run:
|
@galderz good point. would a |
@vsevel Yeah, it would be an anti-pattern and would likely perform worse on JVM mode, aside from being a pain having to change all those |
This is most likely triggered because all |
I actually don't see us doing that in the processor |
oh I confused So I was looking for those, because of what Stuart said made me curious: I've never seen GraalVM include extra code just because it's implementing an interface w/o some specific reason; most often code is included only if the constructor is reacheable, or the class is registered for reflection. But it's an interesting theory, we certainly have seen it become a little more sloppy with dead code elmination for the sake of making more code work OOB. |
It's also possible the code I am using to do the analysis of the file has a bug, but I don't think so. I am thinking about polishing it a bit and adding it to Dev UI, to hopefully allow you to do this sort of analysis in the browser. |
Remember also that GraalVM is able to automatically register for reflection a class when the code is straight forward to analyze. So for example if you just do load a class by class reference - and the parameter is a constant, or a trivial enough function which evaluates to a constant, it will register the class for reflection without us needing to do so explicitly. This might be relevant, because when a class is registered for reflection then it seems to lose the context: the registered class becomes a "possible reasonable candidate" in every call point of its interfaces are invoked on - such as like @stuartwdouglas notices in this case. Might be useful to look into the source code to find possible cases which trigger the automatic reflection? Or trace the reflective registrations, but include the ones automatically registered by GraalVM. I had done such things by blatantly adding some system.out lines to GraalVM but it's been a while. In summary, in such cases we need to trace why the constructor is reachable (either by direct invocation somewhere, or reflections - automatic or implicit). |
I don't think it is registered for reflection though, I can't see it in the report. |
@stuartwdouglas which report are you looking at? |
If this of any interest for this discussion, I did again the builds with
If you need me to produce a report for a given configuration, I will be happy to do so. |
I forgot to mention that with the |
thanks @vsevel , great feedback. We'll keep watching this and see what can be improved further; in particular we'll talk with the GraalVM team about the extra overhead caused by methodhandles but I'm not sure if they can do something about it. What Stuart mentiones is also interesting and we'll keep digging, but it's good to know that it's not critically urgent for you. |
I guess an open question that we'll need to discuss is if Quarkus releases should be built with |
@vsevel @Sanne Re: string concat - I created a small reproducer and was able to get about 322 instances of the |
|
That's not a done thing. We build the Core artifacts on July 22nd so it will depend how fast we can build the images and get everything merged. |
Also, some caveats about GraalVM 21.2. A few weeks back we discovered that it can increase compilation times and memory usage on memory constrained envs (see the great detective work by @zakkak here). @Foivos has added a patch to Quarkus in this respect (see here). As a result, if you want to try GraalVM 21.2, it's best to use a more recent version than Quarkus 2.0.x. |
Agreed, it's highly unlikely to have everything in place on the 22nd. |
a quick note about the initial results I posted when I opened the issue: the test on |
GraalVM 21.2.0 was just released. I was wondering if you could try your application with Quarkus 2.0.x and compare the results of time and memory with 21.2.0 and 21.1.0? One caveat: I'm unsure if Quarkus 2.0.x will work with 21.2.0. As mentioned above, support for GraalVM 21.2.x is expected for Quarkus 2.1.x. |
I removed the note: I did not test the app. I just compiled it.
|
Any chance you could test out this PR: #19115 It should help a lot (assuming it actually works, but the kube client tests passed for me locally). With the integration tests it halved the binary size and more than halved the compile time. |
hello @stuartwdouglas . here are the results. all tests done with
|
So not as big a gain as we have seen elsewhere, but still a nice win. |
I've been doing some benchmarking of Quarkus getting started app (the HTTP REST one) with different GraalVM versions, including latest master commits, and while I was away graalvm master appears to have made considerably improvements to memory consumption: 21.3-dev looks like this today: The memory consumption is still higher than 20.3.3, but 21.3-dev is (in an unconstrained env), the fastest version for building Quarkus getting started app: 21.3-dev was not looking this good before I went away, so I'll be combing through the commits to see what's changed. I'm sure @Sanne will enjoy the gory details :) |
One more detail from the screenshots above: my testing corroborates the big regression found by @vsevel above with 21.1.0, which is the slowest version of the ones I compared. |
Is this still an issue? Especially now that GraalVM has moved to |
probably not. question is: should I remove |
That would be nice, especially using GraalVM 21.3
If you don't want to wait for 2.5 you could give 2.4.1 a go with 21.3, just make sure you use |
I did as you suggested. with 21.3, I see the same build time with or without option
builds were done using |
Describe the bug
I am seeing increased graalvm compilation times between
1.13.3
and2.0
I have launched multiple builds and I see consistency in the results.
I am building both branches on the same host, and with the same command line args.
Here is the output for
1.13
:and for
2.0
:the compile phase went from
73 secs
to461 secs
.I am seeing that we have moved from graalvm
21.0
to21.1
.once the image is built, both versions work identically.
I did not see anything in the migration guide that would require some config change on the graalvm buil.
I am surprised to see that kind of difference. I suppose a lot of people would have noticed. still, since I am using the same host for all quarkus builds, and I do not see anything that could have changed beside the graalvm version.
I have actually restarted a new branch on my project based on my 2.0 quarkus branch, and moved the quarkus version back to
1.13
, and just removed all my tests and commented out the compilation errors, and I get back the expected durations:I have moved back to quarkus
2.0.0.Alpha1
, which uses graalvm21.0
and got normal performances:Expected behavior
similar compilation response time.
Actual behavior
huge increase on the graalvm compilation phase.
To Reproduce
I can reproduce it consistently on my application in my build env. I have not tried to do it on a different simpler app.
Output of
uname -a
orver
OS name: "linux", version: "3.10.0-1160.25.1.el7.x86_64", arch: "amd64", family: "unix"
Output of
java -version
Java version: 11.0.10, vendor: AdoptOpenJDK, runtime: /opt/java/openjdk
GraalVM version (if different from Java)
Quarkus version or git rev
Build tool (ie. output of
mvnw --version
orgradlew --version
)Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
The text was updated successfully, but these errors were encountered: