EnhancedQueueExecutor could disable queue limits to reduce the queue size update overhead

[franz1981]

Description

The quarkus configuration guide state that most users should (could) have an unbounded queue size for the worker thread pool and indeed the default jboss-thread queue size is Integer.MAX_VALUE.
Due to an implementation detail, the queueSize information in jboss-thread is shared both among producer(s) and consumer(s) and there's no way to avoid the overhead (and its scalability implications) with the default settings, which still keep on updating the queue's size even if there's no practical limit to it.

I would love to add a new jboss thread builder configuration i.e. unlimitedQueue(boolean) which disable the queue limit bookeeping per executor, instead of being a global system property (which can still be kept, but could be used as a default or a forced limit too, IDK): this, given that we don't expose MX metrics of our worker, would enable most users to benefit of this improvement, for free - given that most users don't limit queue size as suggested by our doc.

It's important to explain the context of this enhancement, first:

1. users which write PoC of quarkus often (not always!) forget to mark not blocking endpoints with the proper NonBlocking annotation - eg. the typical plaintext hello world endpoint, in short
2. these would run in scalability issues because often the number of event loop is +1 and both event loops and the worker threads compete vs both the head, tail (which is fine) and this additional queue's size counter.
3. these users would strangely benefit from using native image, because native image peak performance for the event loop and the blocking endpoint is slower and this naturally "pad" the attempt to contend against the mentioned shared fields, allowing to scale.

In this weird scenario usually native image tends to scale better (!!!) than JVM mode, but clearly is a quirk of a different problem.

Just for information/reference, in JCTools we track the size using 2 separate fields, instead of one, exactly for this reason
i.e. producerSequence (a long value) and consumerSequence (still a long value) and an additonal producerLimit (still a long)

This allow the producer(s) to:

• check if the current producerSequence exceed the expected limits from producerLimit
• if it exceed it force to get an up-to-date value of the consumerSequence to know the real remaining capacity
• if the new value allows to make some progress, update the producerLimit (which is shared among producers, but doesn't need to be accurate - doesn't need a volatile set nor a compareAndSet, really) and just add the new element if the producerSequence can be moved forward successfully

This mechanism allow to reduce the sharing of information to the bare minimum to make progress - and usually deliver a very high speedup for in-memory and non-blocking tasks.

I know that's weird use case but if it can be a single liner:

• make NO_QUEUE_LIMIT a builder property
• just set it by default if the queue size is Integer.MAX_SIZE

it seems easy and no-brain enough to do it.

@dmlloyd @cescoffier

Implementation ideas

No response

[quarkus-bot]

/cc @Sanne (core), @aloubyansky (core), @gsmet (core), @radcortez (core), @stuartwdouglas (core)

[dmlloyd]

My 2¢ is that we should treat Integer.MAX_VALUE as "unlimited" and hide the change completely inside the thread pool implementation. +1 from me though, it's always good to see a little bit more performance get squeezed out of EQE.

[franz1981]

I'm concerned of this case @dmlloyd :

• the queue is configured with Integer.MAX_VALUE maximumQueueSize - and we disable bookeeping of queueSize
• given that EnhancedQueueeExecutor::setMaximumQueueSize allows, at runtime, to further reduce the queue limits, we cannot enable tracking of queue size, once it was disabled - unless we become empty (which is a condition much easier to verify) - but concurrency wise is not so obvious how to make it right

To me, this seems that we really need a specific builder option to clearly state that user doesn't care about limiting the queue - and this will make setMaximumQueueSize a nop or even throwing an UnsupportedOperationExeption

Let me knwow wdyt

[dmlloyd]

Okay, yeah that makes sense. Thanks!

[franz1981]

Now that jbossas/jboss-threads#190 is merged (thanks @dmlloyd !) waiting for a new release to add the configuration option to quarkus yeeee

[franz1981]

@cescoffier @gsmet
Together with @dmlloyd we improved a bit jboss-threads - David already roll the new release so there are few changes required in Quarkus to pick it (one executor builder change AFAIK) - it's fine for you?

[cescoffier]

That's the perfect timing (after the LTS), so +1.