Assign a distinct event loop to each virtual thread verticle instance - #6313
Open
jnbdz wants to merge 1 commit into
Open
Assign a distinct event loop to each virtual thread verticle instance#6313jnbdz wants to merge 1 commit into
jnbdz wants to merge 1 commit into
Conversation
Motivation: Deploying a verticle with setInstances(N) and ThreadingModel.VIRTUAL_THREAD pins every instance to the same event loop, so raising the instance count does not scale I/O: the single event loop saturates while the virtual threads starve. Vert.x 4.x assigned an event loop per instance via eventLoopGroup.next(). The shared event loop was introduced for worker verticle deployments, where the work happens on the worker pool and a single event loop consumes fewer resources. The virtual thread model was changed at the same time, although that rationale does not apply to it. Changes: Build a fresh context for each virtual thread instance so that each one round-robins over the event loop group, like the event-loop model does. The shared event loop remains for worker deployments.
This file contains hidden or 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
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.
Motivation
Deploying a verticle with
setInstances(N)andThreadingModel.VIRTUAL_THREADpins every instance to the same event loop, so raising the instance count does not scale I/O — the single event loop saturates while the virtual threads starve. The reporter measured a hard ceiling of ~40k RPS withsetInstances(300), against ~180k RPS when the instances were spread over the event loop group.Fixes #5924
Why this looks unintended rather than by design
The shared event loop comes from 7045785, which changed
WORKERandVIRTUAL_THREADin the same diff. Its rationale is stated entirely in terms of worker verticles:That reasoning holds for workers, where the work is dispatched to the worker pool. It does not transfer to the virtual thread model, where the event loop still carries the I/O for everything the verticle creates and there is no separate pool absorbing the load. The virtual thread branch appears to have been carried along with the worker change rather than considered on its own.
For reference, 4.x built a context per instance for both models, each picking
eventLoopGroup.next().Change
Build a fresh context for each virtual thread instance, so each one round-robins over the event loop group like the event-loop model already does.
workerLoopis now used only by theWORKERbranch, which keeps its current behaviour and itstestWorkerInstancesUseSameEventLoopThreadassertion.Test
io.vertx.tests.virtualthread.DeploymentTest#testInstancesUseDistinctEventLoopThreadsdeploys 4 virtual thread instances over an 8 loop pool and asserts 4 distinct event loop threads. It fails on the current code (expected:<4> but was:<1>) and passes with the change. The pool is sized from the instance count so the assertion does not depend on the host's core count.Verified on JDK 21: the full
vertx-core-java21-testsmodule (241 tests), plusDeploymentTest(73),ContextTest(66) andVertxTest(19) invertx-core, all green.Note
@vietj you suggested on the issue that this could be made configurable, and the commit above anticipated "a setting to control this". This PR restores the scaling default for the virtual thread model only, on the reading that the worker rationale never applied to it. If you would rather express it as a
DeploymentOptionssetting — defaulting either way — I am happy to rework it in that direction.This supersedes #6241, which proposed the same change and was closed by its author.