search

quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.28k stars 2.54k forks source link

Upgrade to Jandex 3.2.0 and use the Jandex annotation overlay #40684

Closed Ladicek closed 3 weeks ago

Ladicek commented 4 weeks ago
Ladicek commented 4 weeks ago

Added @mkouba and @manovotn to review the ArC commits and @geoand and @FroMage to review the RESTEasy Reactive commit.

quarkus-bot[bot] commented 4 weeks ago

:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7120e276ddf640bdf9b3916fd79ff04e09d5c117.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

quarkus-bot[bot] commented 3 weeks ago

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 68dc8eb6a6d727cca58e4efc8875c2aef5e2964d.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Sanne commented 3 weeks ago

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.

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 :)

Ladicek commented 3 weeks ago

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.

mkouba commented 3 weeks ago

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.

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 :)

It's not a blocker but in my exprience, if we don't do it now, we never will ;-).

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.

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...

Ladicek commented 3 weeks ago

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 mvn clean package and looked at

[io.quarkus.deployment.QuarkusAugmentor] Quarkus augmentation completed in 1909ms

In current main branch, the reported time was nearly always between 1850 and 1900 ms, with several outliers between 1800 and 1850 ms and also between 1900 and 1950 ms.

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 MAVEN_OPTS="-XX:StartFlightRecording=filename=xxx.jfr,settings=profile,dumponexit=true" mvn clean package 5 times for current main and 5 times for this PR. I then loaded all the JFR files in JMC and saw virtually no difference. The allocation patterns, the GC patterns, the amount of used memory, no significant change in any of those.

mkouba commented 3 weeks ago

In current main branch, the reported time was nearly always between 1850 and 1900 ms, with several outliers between 1800 and 1850 ms and also between 1900 and 1950 ms.

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.

Seems to be an acceptable drop. Did you try the jandex 3.2.0 or the main branch (including https://github.com/smallrye/jandex/commit/29acecceca1c3f75a98137f853cef23e70fe9109 ;-).

To measure memory, I ran MAVEN_OPTS="-XX:StartFlightRecording=filename=xxx.jfr,settings=profile,dumponexit=true" mvn clean package 5 times for current main and 5 times for this PR. I then loaded all the JFR files in JMC and saw virtually no difference. The allocation patterns, the GC patterns, the amount of used memory, no significant change in any of those.

Cool.

Ladicek commented 3 weeks ago

For memory, I only tried Jandex 3.2.0. For time, I tried Jandex 3.2.0 and Jandex main branch with an additional change (no SENTINEL, always store the set of annotations into the map), and I didn't see a difference.