refactor: replace old MPSC/SPSC queues with JCTools (simplified) #1434
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jponge
changed the title
jponge
changed the title
jponge
force-pushed
the
feat/use-jctools-no-native-handling
branch
2 times, most recently
from
40f5592 to
7de1c9e
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1434 +/- ##
============================================
- Coverage 89.34% 89.21% -0.13%
+ Complexity 3365 3270 -95
============================================
Files 459 456 -3
Lines 13434 13198 -236
Branches 1635 1621 -14
============================================
- Hits 12002 11775 -227
+ Misses 808 791 -17
- Partials 624 632 +8
|
|
JCTools 4.0.2 has been released so we can go back to the upstream |
jponge
added
enhancement
jponge
force-pushed
the
feat/use-jctools-no-native-handling
branch
from
781ae68 to
7c9807b
Compare
jponge
force-pushed
the
feat/use-jctools-no-native-handling
branch
from
7c9807b to
50d840a
Compare
- Refactored the Queues class and made all queue creations converge to it (except for tests). - We use the unpadded variants to relieve memory pressure. Issue: #1330 BREAKING CHANGE: constants and methods have been removed from io.smallrye.mutiny.helpers.queues.Queues as well as previous MPSC/SPSC classes in the io.smallrye.mutiny.helpers.queues package.
jponge
force-pushed
the
feat/use-jctools-no-native-handling
branch
from
50d840a to
9c17a6e
Compare
|
This is ready for review 👍 I found breaking changes to Smallrye Reactive Messaging, the fix is easy: jponge/smallrye-reactive-messaging@443a925 |
jponge
changed the title
|
Also @franz1981 if you could do one more review, that'd be great 🙏 |
cescoffier
approved these changes
Jan 23, 2024
No but I'd be happy to 🤣 |
|
👍 I'll take a look for reactive messaging |
ozangunalp
approved these changes
Jan 24, 2024
I like that 🤣 Thanks Ozan |
|
It is the new "it works on my machine" |
|
Note to others: I'll merge myself (I'm looking at Quarkus tweaks for native support) |
|
oki @jponge I didn't had yet the chance to re-review it, sorry :( |
|
I'm struggling with the support of unpadded variants in native. I ended up with the following Quarkus build items -- they are in fact symmetric to those from the Quarkus Netty extension (but shaded and on the base queue variants): new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueProducerIndexField",
"producerIndex"),
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueProducerLimitField",
"producerLimit"),
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueConsumerIndexField",
"consumerIndex"),
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueColdProducerFields",
"producerLimit"),
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueProducerFields",
"producerIndex"),
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueConsumerFields",
"consumerIndex"));Writing an integration test I end up segfaulting on: with the following notable compilation warning:
Unless someone sees an easy fix, we could either:
WDYT? |
|
We would need to measure the overhead. If acceptable, I would go
Unsafe-free.
…On Wed 24 Jan 2024 at 18:03, Julien Ponge ***@***.***> wrote:
I'm struggling with the support of unpadded variants in native.
I ended up with the following Quarkus build items -- they are in fact
symmetric to those from the Quarkus Netty extension (but shaded and on the
base queue variants):
new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueProducerIndexField",
"producerIndex"),new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueProducerLimitField",
"producerLimit"),new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.MpscUnpaddedArrayQueueConsumerIndexField",
"consumerIndex"),new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueColdProducerFields",
"producerLimit"),new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueProducerFields",
"producerIndex"),new UnsafeAccessedFieldBuildItem(
"org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueueConsumerFields",
"consumerIndex"));
Writing an integration test I end up segfaulting on:
Stacktrace for the failing thread 0x0000000131304200 (A=AOT compiled, J=JIT compiled, D=deoptimized, i=inlined):
A SP 0x00000001702129d0 IP 0x000000010100d618 size=64 org.jctools.queues.unpadded.BaseMpscLinkedUnpaddedArrayQueue.offer(BaseMpscLinkedUnpaddedArrayQueue.java)
A SP 0x0000000170212a10 IP 0x000000010100f658 size=48 org.jctools.queues.unpadded.MpscUnboundedUnpaddedArrayQueue.offer(MpscUnboundedUnpaddedArrayQueue.java:27)
A SP 0x0000000170212a40 IP 0x000000010062fb80 size=96 io.quarkus.it.mutiny.nativejctools.MyResource.lambda$createQueues$0(MyResource.java:28)
A SP 0x0000000170212aa0 IP 0x000000010062eaf8 size=48 io.quarkus.it.mutiny.nativejctools.MyResource$$Lambda$9d7ef37a889a56d8a9dcf67834116cfa6fc37755.accept(Unknown Source)
A SP 0x0000000170212ad0 IP 0x000000010096be64 size=96 java.lang.Iterable.forEach(Iterable.java:75)
A SP 0x0000000170212b30 IP 0x000000010062f0c8 size=96 io.quarkus.it.mutiny.nativejctools.MyResource.createQueues(MyResource.java:26)
A SP 0x0000000170212b90 IP 0x000000010062ec5c size=48 io.quarkus.it.mutiny.nativejctools.MyResource$quarkusrestinvoker$createQueues_a4780c8b1dd461d14b3978d89b401e4e442b7033.invoke(Unknown Source)
A SP 0x0000000170212bc0 IP 0x0000000100ff55f0 size=64 org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
A SP 0x0000000170212c00 IP 0x0000000100635c58 size=48 io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)
A SP 0x0000000170212c30 IP 0x0000000100fbd020 size=112 org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
A SP 0x0000000170212ca0 IP 0x00000001006da3bc size=96 io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:582)
A SP 0x0000000170212d00 IP 0x00000001010045c0 size=80 org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
A SP 0x0000000170212d50 IP 0x000000010100545c size=112 org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
i SP 0x0000000170212dc0 IP 0x000000010100af14 size=48 org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
A SP 0x0000000170212dc0 IP 0x000000010100af14 size=48 org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
A SP 0x0000000170212df0 IP 0x0000000100573324 size=48 io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
i SP 0x0000000170212e20 IP 0x00000001009b0eb0 size=48 java.lang.Thread.runWith(Thread.java:1596)
A SP 0x0000000170212e20 IP 0x00000001009b0eb0 size=48 java.lang.Thread.run(Thread.java:1583)
A SP 0x0000000170212e50 IP 0x0000000100242594 size=64 com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:833)
A SP 0x0000000170212e90 IP 0x00000001001f2b0c size=48 com.oracle.svm.core.posix.thread.PosixPlatformThreads.pthreadStartRoutine(PosixPlatformThreads.java:211)
A SP 0x0000000170212ec0 IP 0x000000010015681c size=272 com.oracle.svm.core.code.IsolateEnterStub.PosixPlatformThreads_pthreadStartRoutine_38d96cbc1a188a6051c29be1299afe681d67942e(IsolateEnterStub.java:0)
with the following notable compilation warning:
Warning: RecomputeFieldValue.ArrayIndexScale automatic substitution
failed. The automatic substitution registration was attempted because a
call to jdk.internal.misc.Unsafe.arrayIndexScale(Class) was detected in the
static initializer of org.jctools.util.UnsafeRefArrayAccess. Detailed
failure reason(s): Could not determine the field where the value produced
by the call to jdk.internal.misc.Unsafe.arrayIndexScale(Class) for the
array index scale computation is stored. The call is not directly followed
by a field store or by a sign extend node followed directly by a field
store.
Unless someone sees an easy fix, we could either:
1. wait for JCTools/JCTools#387
<JCTools/JCTools#387> from @franz1981
<https://github.com/franz1981> and a new JCTools release to get
unpadded / Unsafe-free variants, or
2. just switch to the Unsafe-free variants that are padded, provided
the memory overhead is reasonable, and eventually take advantage of
unpadded Unsafe-free variants in the future
WDYT?
—
Reply to this email directly, view it on GitHub
<#1434 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADCG7LSWNTQ65ZTHXVOZXLYQE5FRAVCNFSM6AAAAAA7P3XBQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYGU2TOMJTGU>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Ok, so I might have a fix 🤣 I get all used queues working with just: new UnsafeAccessedFieldBuildItem(
"org.jctools.util.UnsafeRefArrayAccess",
"REF_ELEMENT_SHIFT")and not doing anything special for Go figure.... |
|
Note: I'm on |
|
Hum... we will need to try with GraalVM EE (the non ce version), as it is slightly more picky about these things. |
Works with |
|
See https://github.com/jponge/quarkus/tree/scratchpad/mutiny-with-jctools for the Quarkus impact |
|
I met today with @franz1981
|
jponge
added a commit
to jponge/quarkus
that referenced
this pull request
Jan 26, 2024
… queues GraalVM now seems to have good heuristics to spot the classic Unsafe usage patterns. I removed some UnsafeAccessedFieldBuildItem and added one for UnsafeRefArrayAccess as this class has a problematic static field (REF_ELEMENT_SHIFT). For there record I found about this while working on smallrye/smallrye-mutiny#1434
|
Let's go 😄 |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An alternative to #1413 with no handling of native compilation issues which we know how to handle in Quarkus (see the Netty extension).