-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Upgrade to Jandex 3.2.0 and use the Jandex annotation overlay #40684
Conversation
Ladicek
commented
May 16, 2024
- https://smallrye.io/blog/jandex-3-2-0/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it!
I only looked at the commit changing RESTEasy Reactive
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nice code cleanup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at non-BCE changes in ArC and it looks good! But it would be nice to have some numbers (in terms of memory consumption and build time) for a medium-sized application so that we can be sure there's no regression - after all, it is used extensively.
independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AnnotationStore.java
Show resolved
Hide resolved
7120e27
to
68dc8eb
Compare
Status for workflow
|
Agreed it would be great to verify metrics - but would you ask that as a blocker @mkouba ? Just wondering about the status here, as I was eager to hit the merge button :) |
I'm trying to get some numbers as we speak, actually :-) It's not easy, because a Quarkus build does a lot of other things, so there's a lot of noise. I'll try to write a microbenchmark, but I fear it's just gonna end up stress testing the JIT compiler and not produce anything meaningful. |
It's not a blocker but in my exprience, if we don't do it now, we never will ;-).
I was thinking about manually testing something from https://github.com/quarkus-qe/beefy-scenarios and measure the build time/memory consumption. Just to verify there's no significant regression... |
So I took https://github.com/Ladicek/arc-crazybeans/, which has 500 beans, and added the following extensions:
The total number of ArC annotation transformations with these extensions, in the default configuration, is 19. On an external computer (not on my work laptop), which executes very little processes and was configured for performance measurements, I repeatedly ran
In current With this PR, the reported time was nearly always between 1900 and 1950 ms, with several outliers between 1850 and 1900 ms and also between 1950 and 2000 ms. To measure memory, I ran |
Seems to be an acceptable drop. Did you try the jandex 3.2.0 or the main branch (including smallrye/jandex@29acecc ;-).
Cool. |
For memory, I only tried Jandex 3.2.0. For time, I tried Jandex 3.2.0 and Jandex |