Skip to content

Make JobExecutor::run_jobs_async a plain async method#4331

Merged
jedel1043 merged 3 commits intomainfrom
async-job-executors
Jul 11, 2025
Merged

Make JobExecutor::run_jobs_async a plain async method#4331
jedel1043 merged 3 commits intomainfrom
async-job-executors

Conversation

@jedel1043
Copy link
Member

@jedel1043 jedel1043 commented Jul 11, 2025

This PR makes JobExecutor::run_jobs_async a proper async function, which greatly simplifies our API by removing all the ugly lifetimes.
The change that enabled this was to make the trait's receiver a Rc<Self>, which can be moved inside the future returned by the function.

In contrast to #4328, we don't really require async trait objects in this case, since the only thing that the engine does is calling enqueue_job and run_jobs, which are plain old methods. Thus, I excluded the run_jobs_async method from the trait's VTable with a where Self: Sized bound, but the method can still be called by creating the Rc<Queue> executor first, then passing a clone to the ContextBuilder.

@jedel1043 jedel1043 added this to the next-release milestone Jul 11, 2025
@jedel1043 jedel1043 requested a review from a team July 11, 2025 03:45
@jedel1043 jedel1043 added A-Enhancement New feature or request A-API Changes related to public APIs labels Jul 11, 2025
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 50,595 50,595 0
Passed 47,238 47,238 0
Ignored 1,964 1,964 0
Failed 1,393 1,393 0
Panics 0 0 0
Conformance 93.36% 93.36% 0.00%

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

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

Some optional simplification of the code, otherwise LGTM.

@jedel1043 jedel1043 enabled auto-merge July 11, 2025 21:13
@jedel1043 jedel1043 added this pull request to the merge queue Jul 11, 2025
Merged via the queue into main with commit ff448e8 Jul 11, 2025
15 checks passed
@jedel1043 jedel1043 deleted the async-job-executors branch July 11, 2025 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-API Changes related to public APIs A-Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants