Karm / mandrel-integration-tests

Integration tests for GraalVM and its Mandrel distribution. Runs Quarkus, Helidon and Micronaut applications and small targeted reproducers. The focus is solely on native-image utility and compilation of Java applications into native executables.
Apache License 2.0
5 stars 3 forks source link

Build perf app #254

Closed Karm closed 2 months ago

Karm commented 3 months ago

Adds a new build perf app that sports more dependencies, including two databases and tests for all demo functionality. Demo code is present so as the deps are not optimized away.

This PR is a WIP, I just confirmed it work with the oldest Q we work with in the TS regularly. I will add ports presently to this PR.

$ mvn clean verify -DexcludeTags=all -DincludeTags=perfcheck -Ptestsuite -Dtest=PerfCheckTest#testQuarkusMPOrmAwt -Dquarkus.version=2.13.3.Final -Dperf.app.report=true -Dperf.app.endpoint=http://localhost:8080 -Dperf.app.secret.token=manysecretcharacters

...
2024-04-05 12:13:06.875 INFO  [o.g.t.i.u.Uploader] (postBuildtimePayload) POSTing payload to http://localhost:8080/api/v1/image-stats/import?t=22.3.4-final,2.13.3.Final
2024-04-05 12:13:06.933 INFO  [o.g.t.i.u.Uploader] (postBuildtimePayload) POSTing payload to http://localhost:8080/api/v1/image-stats/194002
2024-04-05 12:13:06.947 INFO  [o.g.t.i.PerfCheckTest] (testQuarkusMPOrmAwt) Response code:200
2024-04-05 12:13:06.947 INFO  [o.g.t.i.PerfCheckTest] (testQuarkusMPOrmAwt) Response body:{"id":194002,"created_at":"2024-04-05T10:13:06.907+00:00","tag":"22.3.4-final,2.13.3.Final","img_name":"mp-orm-dbs-awt-runner","generator_version":"GraalVM 22.3.4.0-Final Java 17 Mandrel Distribution","image_size_stats":{"id":194002,"total_bytes":108991144,"code_cache_bytes":52835328,"heap_bytes":52580352,"resources_bytes":1471995,"resources_count":47,"other_bytes":3575464,"debuginfo_bytes":0},"jni_classes_stats":{"id":685005,"classes":178,"fields":1535,"methods":2092},"reflection_stats":{"id":685007,"classes":852,"fields":533,"methods":4464},"build_perf_stats":{"id":194002,"total_build_time_sec":114.003,"gc_time_sec":8.585,"num_cpu_cores":16,"total_machine_memory":66847313920,"peak_rss_bytes":6762926080,"cpu_load":9.896533342654653},"total_classes_stats":{"id":685008,"classes":27794,"fields":62023,"methods":216037},"reachability_stats":{"id":685006,"classes":25604,"fields":40448,"methods":126403}}
jerboaa commented 3 months ago

@Karm I'm a bit concerned that the test suite is growing too big and the burden to get them fixed becomes a problem. Could we perhaps ensure that those performance profiles don't run by default and only run when activated (which should be done on some jobs we run, but definitely not all). Especially in GHA, test failures pop up frequently with new warnings and produce noise (case in point: current test failures in this PR).

Karm commented 3 months ago

Thx @jerboaa for the comment. FYI @zakkak

@Karm I'm a bit concerned that the test suite is growing too big and the burden to get them fixed becomes a problem. Could we perhaps ensure that those performance profiles don't run by default and only run when activated (which should be done on some jobs we run, but definitely not all). Especially in GHA, test failures pop up frequently with new warnings and produce noise (case in point: current test failures in this PR).

This PR is a draft for Quarkus 2.13.3 as the subject of the PR states, i.e. as you can see 2.13.3.Final GA jobs are green here.

Furthermore, this is affecting the perfcheck JUnit tag, and that is not executed at all by default:

   <includeTags>runtimes,reproducers</includeTags>

We enable it here on the CI because here we are "testing the testsuite".

Regarding adding apps: As of this PR, apps/quarkus-full-microprofile is actually a subset of apps/quarkus-mp-orm-dbs-awt, so I think we can consider replacing the former with the latter. The one big change in that would be that the runtimes JUnit tag would suddenly require Podman/Docker so as Testcontainers could run and MariaDB and Postgres could be started. It would be a new requirement, because the test suite does not require containers unless you specifically want container tests enabled by -Ptestsuite-builder-image. Given our environment, I think it's perhaps a safe thing to do to require containers on host by default.

EDIT: Meh. We can't replace one with the other. While apps/quarkus-full-microprofile natively builds on Windows, apps/quarkus-mp-orm-dbs-awt won't until we port Quarkus AWT extension to Windows. So disregard that. I could create submodules though.

jerboaa commented 3 months ago

Furthermore, this is affecting the perfcheck JUnit tag, and that is not executed at all by default:

   <includeTags>runtimes,reproducers</includeTags>

We enable it here on the CI because here we are "testing the testsuite".

Mandrel CI uses this -Ptestsuite: https://github.com/graalvm/mandrel/blob/d9566d884ad80014d430983e846ca01745950a50/.github/workflows/base.yml#L736-L738

So I'd assume it would run this one too?

Karm commented 3 months ago

Furthermore, this is affecting the perfcheck JUnit tag, and that is not executed at all by default:

   <includeTags>runtimes,reproducers</includeTags>

We enable it here on the CI because here we are "testing the testsuite".

Mandrel CI uses this -Ptestsuite: https://github.com/graalvm/mandrel/blob/d9566d884ad80014d430983e846ca01745950a50/.github/workflows/base.yml#L736-L738

So I'd assume it would run this one too?

Nope, see it's tagged perfcheck here. And that tag is not included in the profile here.

jerboaa commented 3 months ago

OK. Thanks. I'm probably confusing it with the JFR perf test then.

Karm commented 3 months ago

OK. Thanks. I'm probably confusing it with the JFR perf test then.

That is right, we wanted those on even for regular runs, so they are annotated as reproducers here.

The fact that it confuses you means I botched the user experience of the tags taxonomy. I guess we can refactor that to some more logical parts.

Karm commented 2 months ago
Karm commented 2 months ago

OK, given that you'll file a quarkus issue on the http components client closing issue. Thanks!

THX for the review. I've bee cooking this one for some days :-)

Ad issue: I'll give it an hour to explore the CDI scope and fixing it in the app code, if it fails, I'll open an issue with a reproducer, linking to the old one.

Karm commented 2 months ago

OK, given that you'll file a quarkus issue on the http components client closing issue. Thanks!

@jerboaa I am working on that in a separate PR. I found out that the problem disappears if wrap in try-with-resources...except for some older versions, where either as a bug or as a feature, the Client isn't Closable and thus you have to call .close() manually in finally block the old way. I am testing it now, but it seems it's just a clumsy API inconsistencies over the rest easy versions in Quarkus that could be dealt with in the user's codebase.

jerboaa commented 2 months ago

I found out that the problem disappears if wrap in try-with-resources

That's what I thought. So we ought to fix the test and then remove the allow-listing.

Karm commented 2 months ago

I found out that the problem disappears if wrap in try-with-resources

That's what I thought. So we ought to fix the test and then remove the allow-listing.

It is happening in #255