Skip to content

Use Virtual Threads when applicable #25855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented May 23, 2025

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label May 23, 2025
@wendigo wendigo requested review from raunaqmorarka and losipiuk May 23, 2025 12:59
@losipiuk
Copy link
Member

Any measurable benefits? Data please :)

@wendigo
Copy link
Contributor Author

wendigo commented May 23, 2025

@losipiuk it's hard to quantify - this will use less resources but we do not track threads in the memory accounting

@@ -109,7 +108,7 @@ public void setup(Binder binder)

newOptionalBinder(binder, Key.get(ExecutorService.class, ForJdbcClient.class))
.setDefault()
.toInstance(newCachedThreadPool(daemonThreadsNamed(format("%s-jdbc-client-%%d", catalogName))));
.toInstance(newThreadPerTaskExecutor(virtualThreadsNamed(catalogName + "-jdbc-client-#%s")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
format("%s-jdbc-client-#%s", catalogName)

@starburstdata-automation
Copy link

starburstdata-automation commented Jun 11, 2025

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_part.

Building Trino finished with status: success
Benchmark finished with status: success
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: NO Regression found.
Benchmark Comparison to the closest run from Master: Report

@wendigo wendigo force-pushed the serafin/virtual-threads branch from bcbeae0 to 0678d52 Compare June 11, 2025 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants