@ozangunalp
I still see a value into having VT named to be recognized as "quarkus" ones.
that help troubleshooting deadlocks or other problems withjstack.

I agree let's remove the trailing dash fromVirtualThreadsConfig#namePrefix default value.

I saw few failing tests before, let's see if I forgot anything.
@ozangunalp should I change the config doc to explain that's not using any counter anymore?
In theory people have thethreadId for that

What does it bring exactly? It makes so much of a difference that we can’t wait for the JDK fix?

What does it bring exactly? It makes so much of a difference that we can’t wait for the JDK fix?

That's a good point. Setting the configVirtualThreadsConfig#namePrefix null should also have the same effect, as it won't name the VTs.

Edit:

Like this :https://quarkus.io/guides/virtual-threads#virtual-thread-names

What does it bring exactly? It makes so much of a difference that we can’t wait for the JDK fix?

Let me show you...

packagered.hat.puzzles.loom;importorg.openjdk.jmh.annotations.*;importjava.util.concurrent.ThreadFactory;importjava.util.concurrent.TimeUnit;importjava.util.concurrent.atomic.AtomicLong;@BenchmarkMode(Mode.AverageTime)@OutputTimeUnit(TimeUnit.NANOSECONDS)@Measurement(iterations =10,time =400,timeUnit =TimeUnit.MILLISECONDS)@Warmup(iterations =10,time =1,timeUnit =TimeUnit.SECONDS)@State(Scope.Thread)@Fork(2)publicclassVirtualThreadThreadCreate {ThreadFactorynamingFactory;ThreadFactorydelegatingFactory;ThreadFactoryprefixOnlyFactory;privateStringprefix;@Setuppublicvoidinit() {prefix ="quarkus-virtual-thread-";namingFactory =Thread.ofVirtual().name(prefix,0).factory();varvtFactory =Thread.ofVirtual().factory();finalAtomicLongcounter =newAtomicLong();delegatingFactory =r -> {Threadt =vtFactory.newThread(r);t.setName(prefix +counter.getAndIncrement());returnt;        };prefixOnlyFactory =Thread.ofVirtual().name(prefix).factory();    }@BenchmarkpublicThreadcreateWithNamingFactory() {returnnamingFactory.newThread(() -> {        });    }@BenchmarkpublicThreadcreateWithDelegatingFactory() {returndelegatingFactory.newThread(() -> {        });    }@BenchmarkpublicThreadcreateWithPrefixOnlyFactory() {returnprefixOnlyFactory.newThread(() -> {        });    }}

we got:

Benchmark                                                                 Mode  Cnt      Score     Error   UnitsVirtualThreadThreadCreate.createWithDelegatingFactory                     avgt   20     29.179 ±   0.964   ns/opVirtualThreadThreadCreate.createWithDelegatingFactory:gc.alloc.rate.norm  avgt   20    344.001 ±   0.001    B/opVirtualThreadThreadCreate.createWithNamingFactory                         avgt   20     37.591 ±   0.411   ns/opVirtualThreadThreadCreate.createWithNamingFactory:gc.alloc.rate.norm      avgt   20    432.001 ±   0.001    B/opVirtualThreadThreadCreate.createWithPrefixOnlyFactory                     avgt   20     13.301 ±   0.203   ns/opVirtualThreadThreadCreate.createWithPrefixOnlyFactory:gc.alloc.rate.norm  avgt   20    272.000 ±   0.001    B/op

which shows that the version without counter is not only faster but cheaper in term of allocations.
The other point is: "it really help users havinganother id if you already can read thethread-id which is unique as well?"
Having the name which state that such VT are quarkus related, is good, but providing another counter, not really, IMHO

🎊 PR Preview5e1c2f0 has been successfully built and deployed tohttps://quarkus-pr-main-51223-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

@franz1981 I'm afraid there are several tests that will need to be adapted

@Sanne

Thanks, looking into it

@Sanne@ozangunalp

While waiting the status of the tests, I see these options here:

1. dropping thename-prefix config as a whole, and just replace it withname: Thread(s) can already be identified by theirThread::getId, on logging, whilst thename allow users to distinguish which one are related Quarkus and which not.
2. leave it as it is, with a workaround: seeStop using named (with counter) VT names due to https://bugs.openjdk.org/browse/JDK-8372410 #51223 (comment)createWithDelegatingFactory; low risk (no test changes), the performance impact is mitigated, although it's still allocating and costing more than the option 1.

I have to be honest that although I'm usually all in for low risk changes, Loom adoption is still at early stages, and I prefer to not maintaining features which imply a cost on our side and no improvement for the life of users.

Status for workflowQuarkus Documentation CI

This is the status report for runningQuarkus Documentation CI on commit44e3ef9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

I like option 1

+1. Let's go with option 1.

I actually liked the fact that virtual threads have a name with a unique number too. And I would showcase that with some demos in my presentations too 😄 This is quite useful when you are inspecting what is happening and to check where you code is executed.

Just build this branch locally and with this change everything that runs on a virtual threads shows asquarkus-virtual-thread for both normal logs and access logs. And I'm not able to identify if it's the same virtual thread or a different one. This is especially useful if you do work in parallel and want to see what happens. Also later if you use structured concurrency.

And I'm not able to identify if it's the same virtual thread or a different one. 

This looks more a problem of logging instead, which shouldn't rely on thread names but on thread ids

FYI
https://github.com/openjdk/loom/blob/c2d6b72d0997615894ecca1ad8489027a7e61138/src/java.base/share/classes/java/lang/VirtualThread.java#L1376-L1380

Thread.toString is making use of both informations because the name alone, being just a String, is not expected/enforced to identify uniquely them, whilst the thread id has been implemented to do so

@ozangunalp any idea where it happens?

And I'm not able to identify if it's the same virtual thread or a different one.

This looks more a problem of logging instead, which shouldn't rely on thread names but on thread ids

FYIhttps://github.com/openjdk/loom/blob/c2d6b72d0997615894ecca1ad8489027a7e61138/src/java.base/share/classes/java/lang/VirtualThread.java#L1376-L1380

Thread.toString is making use of both informations because the name alone, being just a String, is not expected/enforced to identify uniquely them, whilst the thread id has been implemented to do so

@ozangunalp any idea where it happens?

If we somehow can make the logging output the thread id that would be good I think to be able to correctly identify them.

Status for workflowQuarkus CI

This is the status report for runningQuarkus CI on commit44e3ef9.

Failing Jobs

Full information is available in theBuild summary check run.
You can consult theDevelocity build scans.

Failures

⚙️ JVM Tests - JDK 21#

- Failing: extensions/panache/hibernate-orm-rest-data-panache/deployment

📦 extensions/panache/hibernate-orm-rest-data-panache/deployment

❌io.quarkus.hibernate.orm.rest.data.panache.deployment.build.BuildConditionsWithResourceDisabledTest. -History -More details -Source on GitHub

java.lang.RuntimeException: Failed to start quarkusat io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)at io.quarkus.runtime.Application.start(Application.java:101)at java.base/java.lang.reflect.Method.invoke(Method.java:580)at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:350)at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:703)at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)Caused by: java.lang.RuntimeException: java.lang.OutOfMemoryError: Metaspace

⚙️ JVM Integration Tests - JDK 21 Semeru#

- Failing: integration-tests/virtual-threads/grpc-virtual-threads

📦 integration-tests/virtual-threads/grpc-virtual-threads

❌io.quarkus.grpc.example.streaming.VertxVirtualThreadTest.testUnary -History -More details -Source on GitHub

io.grpc.StatusRuntimeException: INTERNAL: Half-closed without a requestat io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:368)at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:349)at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:174)at io.grpc.testing.integration.TestServiceGrpc$TestServiceBlockingStub.unaryCall(TestServiceGrpc.java:277)at io.quarkus.grpc.example.streaming.VirtualThreadTestBase.testUnary(VirtualThreadTestBase.java:33)at java.base/java.lang.reflect.Method.invoke(Method.java:586)at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:995)

❌io.quarkus.grpc.example.streaming.VirtualThreadTest. -History -More details -Source on GitHub

org.junit.jupiter.engine.execution.ConditionEvaluationException: Failed to evaluate condition [io.quarkus.test.junit.QuarkusTestExtension]: Internal error: Test class was loaded with an unexpected classloader (QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST for JUnitQuarkusTest-no-profile (QuarkusTest)@81f4b324) or the thread context classloader (jdk.internal.loader.ClassLoaders$AppClassLoader@430764a7) was incorrect.at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1685)at java.base/java.util.stream.Referen...

❌io.quarkus.grpc.example.streaming.VirtualThreadTest.testGrpcClient -History -More details -Source on GitHub

java.lang.AssertionError: 1 expectation failed.Expected status code <200> but was <500>.at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486)at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:108)

Flaky tests -Develocity

⚙️ JVM Tests - JDK 25

📦 extensions/smallrye-graphql/deployment

❌io.quarkus.smallrye.graphql.deployment.CompletionStageTest.testSourcePost -History

• 1 expectation failed. Expected status code <200> but was <500>. -java.lang.AssertionError

java.lang.AssertionError: 1 expectation failed.Expected status code <200> but was <500>.at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:483)at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)

📦 extensions/smallrye-reactive-messaging/deployment

❌io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector -History

• Expecting actual: ["-6","-7","-9","-10","-11","-12","-13","-14"] to start with: ["-6", "-7", "-8", "-9"] -java.lang.AssertionError

java.lang.AssertionError: Expecting actual:  ["-6","-7","-9","-10","-11","-12","-13","-14"]to start with:  ["-6", "-7", "-8", "-9"]at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:41)

⚙️ JVM Tests - JDK 17 Windows

📦 extensions/micrometer-opentelemetry/deployment

❌io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_AsyncFailed -History

• Stream has no elements -java.lang.IllegalArgumentException

java.lang.IllegalArgumentException: Stream has no elementsat io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lambda$lastReading$2(MetricDataFilter.java:213)at java.base/java.util.Optional.orElseThrow(Optional.java:403)at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReading(MetricDataFilter.java:213)at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReadingDataPoint(MetricDataFilter.java:231)at io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_AsyncFailed(MicrometerTimedInterceptorTest.java:150)at java.base/java.lang.reflect.Method.invoke(Method.java:569)at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:532)

⚙️ MicroProfile TCKs Tests

📦 tcks/microprofile-lra

❌org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable -History

• Expecting the metric Compensated callback was called Expected: a value equal to or greater than <1> but: <0> was less than <1> -java.lang.AssertionError

java.lang.AssertionError: Expecting the metric Compensated callback was calledExpected: a value equal to or greater than <1>     but: <0> was less than <1>at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)at org.eclipse.microprofile.lra.tck.TckRecoveryTests.assertMetricCallbackCalled(TckRecoveryTests.java:210)at org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable(TckRecoveryTests.java:195)at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

The issue is most users already use the thread name in their logs to identify threads.

I think we accept that downside, and can add to the documentation that VTs can still be identified via thread ids.

Just build this branch locally and with this change everything that runs on a virtual threads shows asquarkus-virtual-thread for both normal logs and access logs. And I'm not able to identify if it's the same virtual thread or a different one.

Thanks@wjglerum ! I think this is really important feedback - I naively assumed from@franz1981 's comments above about usingthreadId that loggers would already do it , my bad.

I agree this would be a substantial set back if existing log formats wouldn't be able to distinguish two different threads - in this case my preference changes, I don't think we can break that - error diagnostics needs to remain useable.

I see two options:

• adapt our logging system as part of this PR (and possibly logging formats), & document the need to adapt custom formats
• switch to the other alternative, at least for now

Due to the better and correct usage of thread Ids I would vote for

adapt our logging system as part of this PR (and possibly logging formats), & document the need to adapt custom formats

No idea where loggings happen - any pointer?

@franz1981 After all these discussions, I think we should go with the custom delegating thread factory approach.

No idea where loggings happen - any pointer?

We propose default formats for logging, which are configured viaLogRuntimeConfig.
In that, the Jboss logging format allows including thread name with%t and thread id with%t{id}.

In that, the Jboss logging format allows including thread name with %t and thread id with %t{id}.

so my idea is to just perform the composition myself there...e.g. concatenating the thread name with the thread id there.
I think is the right thing to do regardless because will work well for platform threads as well.
If you perform a jstack dump you would see thrread id you want to correlated with logs, not just thread names, which are not granted to be unique.

I agree that it is sensible to include the thread id in the logs. But the user can already do that by configuring the log format.

I think what we should do in this PR is :

• Change to a delegating thread factory with a proper thread name counter (Maybe we can use the threadId in that counter ?)
• Add the option to disable the thread counter and only set the VT name

So we keep the current generally recognized behaviour, minimise the impact of the JDK bug, and give the option to name threads without counter and rely on the thread id.

Other thread factories in Quarkus do have counter-based thread names: Vertx threads and Jboss worker threads.

I agree on the fact to reduce the impact on users, but since I have the performance hat as well, It's important I remember that what we do for other cases which involve a scarcely allocated resources is not relevant here (UX apart): virtual threads are meant to be allocated and never reused.
Over-allocating for each of them (in the benchmark it's about +30% more + improved latency) for something which could be delivered via logging tuning (which we can perform outselves) is not the best option we could strive for.
Said that, if you prefer to leave this perf boost here (there's no JDK fix for that) I will pursue the "delegation" path, instead ☮️

I totally follow your point.

I've been playing with your benchmarks, I expected that if we eliminate the counter and reuse the threadId the impact would be negligible.

I expected that if we eliminate the counter and reuse the threadId the impact would be negligible.

that would happen only if we do it on logging side only, because logging is not always enabled/on, whilst providing such a composed name is a cost we would pay always, upfront, regardless needed or not.
Said that, performance is a funny thing: you feel it miss only when you need it/look at it, but in the grand scheme, nothing of it is key for a functional pov, but a plus (nice one, if you ask me, but I'm biased eheh).

Honestly, on the logging side, I don't see how we can do this upstream on Jboss logging in a general manner.
In Quarkus, we can setup a logging filter to include the thread id in the logged thread name. But it is hacky.

So again, I think I am on the same position as before.
We give the option to disable thread counters, so one can configure for the best of performance.

how we can do this upstream on Jboss logging in a general manner.

got it, so it is more on the jboss logger side; at this point there isn't much we can do, agree.
I'll reword this to reduce the impact, thanks!

so it is more on the jboss logger side; at this point there isn't much we can do, agree.

I'm not following. What's the problem on the JBoss Logger side? If any, we can certainly suggest improvements and get a quick release.

But it's my understanding so far that we should simply be able to change the default pattern, and remove the counter. What am I missing?