Skip to content
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

Use a virtual thread factory #15

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

robotdan
Copy link
Member

Switch to using virtual threads. This will allow us to push much harder on a single system.

Example with 100 threads for login.

Login
- 100 p-threads
                            Total duration:	114054 ms (~114 s)
                            Total count:		100000
                            Avg. latency:		2.052 ms
                            Avg. success:		876.778 per second

- 100 v-threads
                            Total duration:	44351 ms (~44 s)
                            Total count:		100000
                            Avg. latency:		42.931 ms
                            Avg. success:		2254.741 per second

This should let us push FusionAuth a lot harder- at least without having to spin up a bunch of compute to get enough threads to really push it.

The difference is the most evident with a lot of threads because we end up just spending most of the time context switching when we run fusionauth-app and this load test on the same host. FusionAuth is likely then getting starved for CPU time if the load test harness spins up that many platform threads.

This result is the Login API w/ a load factor of 1 on the password hash, so this simulates removing CPU as the bottleneck.

@robotdan robotdan requested a review from a team as a code owner January 28, 2025 19:58
} catch (Exception ignore) {
// Note that we are going to use virtual threads, so in theory we don't need a pool, but we are trying to simulate clients or workers
// so keep the thread pool and just use a virtual factory.
try (ExecutorService pool = Executors.newFixedThreadPool(workers.size())) {
Copy link

Choose a reason for hiding this comment

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

is this supposed to be using newVirtualThreadPerTaskExecutor()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do that on purpose, but I suppose I could.

The goal was to keep the simulated number of workers. In practice, perhaps it is the same since we spin up the number of workers explicitly. Maybe 6 of one half dozen of the other? Perhaps the only benefit of using a pool with the virtual workers is if we want to make the pool smaller than the number of workers which we never do...

So... maybe I should just remove it. I'll take another look.

Copy link

Choose a reason for hiding this comment

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

I'm not seeing how this code is using virtual threads

Copy link
Member Author

Choose a reason for hiding this comment

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

I swapped it out and removed my comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants