Skip to content
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

Improve the number of classes that are loaded by our AppCDS support #28638

Merged
merged 1 commit into from Oct 18, 2022

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Oct 17, 2022

The way this is done is by picking a much later point in the startup process that is safe for the application to reach. This point is basically when Arc has created all its beans as frameworks commence their work after this point.

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/core labels Oct 17, 2022
@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2022

This actually does make a (small) but much more notireable dent in startup time vs our current AppCDS support.

@maxandersen
Copy link
Contributor

Feels related to #28290 or at least could be useful appcds gets recorded at similar time a resume/suspend snapshot happens.

@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2022

Yeah, it's fairly similar, but this is very very targeted

@geoand
Copy link
Contributor Author

geoand commented Oct 18, 2022

@stuartwdouglas can you think of any reason not to make this enhancement?

Copy link
Member

@stuartwdouglas stuartwdouglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the more classes that are loaded the better (assuming these are going to be loaded anyway).

The way this is done is by picking a much later point in the startup process
that is safe for the application to reach. This point is basically when Arc
has created all its beans as frameworks commence their work after this point.
@geoand geoand marked this pull request as ready for review October 18, 2022 08:37
@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 18, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 18, 2022

Failing Jobs - Building b850d5b

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 17 MacOS M1
✔️ JVM Tests - JDK 18

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/vertx/deployment 
! Skipped: extensions/agroal/deployment extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 340 more

📦 extensions/vertx/deployment

io.quarkus.vertx.DuplicatedContextTest.testThatBlockingEventConsumersAreCalledOnDuplicatedContext - More details - Source on GitHub

(RECIPIENT_FAILURE,8185) io.smallrye.mutiny.TimeoutException
	at io.vertx.core.eventbus.Message.fail(Message.java:141)
	at io.quarkus.vertx.runtime.VertxRecorder$3$1$1.handle(VertxRecorder.java:122)

@geoand geoand merged commit 8225e8c into quarkusio:main Oct 18, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 18, 2022
@geoand geoand deleted the better-appcds branch October 18, 2022 13:18
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 18, 2022
geoand added a commit to geoand/quarkus that referenced this pull request Oct 19, 2022
Follow up of quarkusio#28638
as I had managed to forget to commit this part...
tmihalac pushed a commit to tmihalac/quarkus that referenced this pull request Oct 27, 2022
Follow up of quarkusio#28638
as I had managed to forget to commit this part...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/core triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants