Avoid allocating instances of AtomicReference in Uni #660
Conversation
Codecov Report
@@ Coverage Diff @@
## main #660 +/- ##
============================================
+ Coverage 90.03% 90.28% +0.24%
- Complexity 3022 3035 +13
============================================
Files 395 395
Lines 11829 11833 +4
Branches 1478 1480 +2
============================================
+ Hits 10650 10683 +33
+ Misses 604 585 -19
+ Partials 575 565 -10
|
There was a problem hiding this comment.
LGTM,
I tried to avoid field updater to avoid the cost in native (at some point it was adding every classes having a field updater even if not used - that was quite a long time ago when I started Mutiny).
I'm not concerned by this with your changes, as anyway we need this class every time.
|
Mutiny is still on Java 8, right? Otherwise we could use |
|
@Ladicek for now, yes... Java 8. For how long? That's a topic I'm working on currently (not on the technical side). Now that varhandle are supported in native executable, that would be very interesting to compare. |
|
Right, in fact I had started my first experiment with I briefly considered multi-release jars, but the effort of introducing that in the project (and wondering if it would be welcome?) put me off. After all the problem I'm solving here is allocations, and this is effective towards that goal. Having the internals now encapsulated, it should be trivial to switch later on. |
|
Method names are fine, and we can always refactor since it's all internal APIs :-) |
|
It's safe to have an iteration to reduce allocation (granted performance remains flat) with atomic field updaters, and then in the future we can do another iteration to investigate |
jponge
changed the title
|
Part of #666 epic |
|
(the fact that the epic has number 666 is pure coincidence) |
jponge
added
enhancement
|
Starting work, rebasing, I'll push force over here if need be |
|
@Sanne I've done a few minor additions for I did not blindly go through all places in |
|
@Sanne you might cherry-pick that commit, I can't push to your branch |
implementation/src/main/java/io/smallrye/mutiny/operators/uni/UniOperatorProcessor.java
Show resolved
Hide resolved
|
#669 is a clean branch, also includes @MarkMarkyMarkus remark |
|
Nice, should we close this PR? |
|
Yes :-) |
Under load in Quarkus, when profiling a Reactive application (RestEasy Reactive + Hibernate Reactive), I can see a significant amount of
AtomicReferenceinstances being allocated, as internal implementation of eachUniOperatorProcessor.In this PR I'm suggesting to refactor the
AtomicReferenceto use anAtomicReferenceFieldUpdater.The method names are a bit mouthful :) Any better suggestions? I'm patching code I don't know very well, so not sure how to call these.