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

Save AtomicBoolean allocation in Arc #25032

Merged
merged 1 commit into from
Apr 20, 2022
Merged

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Apr 20, 2022

There is no reason to use an AtomicBoolean in this case,
as there aren't any compareAndSwap (or similar) operations,
just plain reading and writing of the field, which means
a volatile boolean is perfectly fine

This showed up in a sample profiling run I did with async-profiler

There is no reason to use an atomic boolean in this case,
as there aren't any compareAndSwap (or similar) operations,
just plain reading and writing of the field, which means
a volatile boolean is perfectly fine
@geoand geoand requested a review from stuartwdouglas April 20, 2022 09:50
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Apr 20, 2022
@geoand geoand requested review from mkouba and removed request for stuartwdouglas April 20, 2022 09:51
@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 20, 2022
@gastaldi gastaldi merged commit 957ed4e into quarkusio:main Apr 20, 2022
@quarkus-bot quarkus-bot bot added this to the 2.9 - main milestone Apr 20, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 20, 2022
@gastaldi gastaldi deleted the arc-allocations branch April 20, 2022 14:35
@gastaldi
Copy link
Contributor

Should this be backported?

@geoand
Copy link
Contributor Author

geoand commented Apr 20, 2022

Yeah, it probably makes sense to backport to 2.7 and 2.8

@gsmet
Copy link
Member

gsmet commented Apr 24, 2022

I haven't backported it as there are conflicts. I think it can wait.

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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants