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

Allow ThreadPool to run jobs in both modes, and improve scheduling #1837

Open
wants to merge 14 commits into
base: 8.3.0-Dev
Choose a base branch
from

Conversation

player-03
Copy link
Contributor

@player-03 player-03 commented Sep 2, 2024

There are enough use cases for both single-threaded and multi-threaded jobs that it's likely some users will want to use both. Before this PR, this required creating two ThreadPools, one for each mode. Now, you only need the one.

If you call run() without passing the new mode argument, it falls back to the pool's mode, producing the same behavior.

I've also updated how jobs are scheduled. Previously, it would call __startJobs() only once per frame. Now, it spins up new threads immediately if it hasn't hit the cap, and when a thread sends a COMPLETE or ERROR event, the main thread will immediately reply with the next job. This is a little bit faster than waiting 1/60th of a second for the next __startJobs() call.

On top of that, it can now run multiple single-threaded jobs per frame, including when one job's completion triggers the next.

@player-03 player-03 changed the title Allow a single ThreadPool to run jobs in both modes Allow ThreadPool to manage both job types, run jobs sooner Dec 11, 2024
@player-03 player-03 linked an issue Dec 11, 2024 that may be closed by this pull request
Since we're storing more and more in `JobData`, and the background thread only needs three bits of that data, I added those to `ThreadEvent` so we don't have to pass the full object. This may improve performance in HTML5 specifically, where passing a class instance incurs an overhead.
Now that thread pools can manage both types of job at once, we can't rely on `mode` to determine whether we're on a background thread. Honestly, I shouldn't have relied on it in the first place.
No need to set `__mainThread` based on `isWorker()` when `isWorker()` already gives the information we're after.
For single-threaded jobs, this means a pool can now handle multiple per frame. For multi-threaded jobs, this only slightly reduces the delay between jobs.
Forgot this when overriding the "send" functions.
@player-03 player-03 force-pushed the multi_mode_ThreadPool branch from 68cc667 to 02d478d Compare December 31, 2024 22:00
It's a private variable, so all it really needs to do is mention the location it gets used.
Plus, alphabetize the variables.

In 8.2.0, a single-threaded pool would report `currentThreads == 1` when running a job, meaning it counted the main thread. But in retrospect, this was both redundant (with `activeJobs`) and unexpected, so I'm counting it as a bug.
All the jobs in `__multiThreadedJobs` are already known to be running in multi-threaded mode. This is left over from when pools were locked to a single mode.
We still need to throw an error when `mode` is `MULTI_THREADED`, but this can now vary per job, so the check must happen during `run()`.

Also, the old error message was out of date. You can't pass a function to the `ThreadPool` constructor.
@player-03 player-03 changed the title Allow ThreadPool to manage both job types, run jobs sooner Allow ThreadPool to run jobs in both modes, and improve scheduling Jan 6, 2025
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.

Future freezing application
1 participant