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

Add customization point for scheduling ExecutorJobs on EventLoops #2538

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

FranzBusch
Copy link
Member

Motivation

Currently, the NIO's EventLoop conformance to the SerialExecutor protocol always uses execute to schedule the actual job. However, the closure for execute has to close over the job and the EventLoop itself; hence, it always allocates. Since jobs are a very fine grained object in Concurrency that are created a lot this lead to millions of allocations in even small benchmarks.

Modification

This PR provides a customization point for EventLoops to execute ExecutorJobs directly. For SelectableEventLoop we store a type erased UnownedJob in our ScheduledTask and just run it right away.

Result

No more allocations when NIO's EL is used as a SerialExecutor.

Sources/NIOCore/EventLoop.swift Outdated Show resolved Hide resolved
Sources/NIOPosix/MultiThreadedEventLoopGroup.swift Outdated Show resolved Hide resolved
Sources/NIOPosix/SelectableEventLoop.swift Outdated Show resolved Hide resolved
@FranzBusch FranzBusch force-pushed the fb-serial-executor-performance branch 2 times, most recently from c14ee8a to b36ad5a Compare October 9, 2023 11:00
@Lukasa Lukasa added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Oct 9, 2023
Sources/NIOCore/EventLoop+SerialExecutor.swift Outdated Show resolved Hide resolved
Sources/NIOCore/EventLoop.swift Show resolved Hide resolved
Sources/NIOPosix/SelectableEventLoop.swift Show resolved Hide resolved
Sources/NIOPosix/SelectableEventLoop.swift Outdated Show resolved Hide resolved
@FranzBusch FranzBusch force-pushed the fb-serial-executor-performance branch from b36ad5a to 8e9d701 Compare October 9, 2023 11:33
@FranzBusch FranzBusch requested a review from Lukasa October 9, 2023 11:33
@FranzBusch FranzBusch force-pushed the fb-serial-executor-performance branch 2 times, most recently from b465a7a to 2ed0c6f Compare October 9, 2023 11:57
# Motivation
Currently, the NIO's EventLoop conformance to the `SerialExecutor` protocol always uses `execute` to schedule the actual job. However, the closure for `execute` has to close over the job and the `EventLoop` itself; hence, it always allocates. Since jobs are a very fine grained object in Concurrency that are created a lot this lead to millions of allocations in even small benchmarks.

# Modification
This PR provides a customization point for `EventLoop`s to execute `ExecutorJob`s directly. For `SelectableEventLoop` we store a type erased `UnownedJob` in our `ScheduledTask` and just run it right away.

# Result
No more allocations when NIO's EL is used as a `SerialExecutor`.
@FranzBusch FranzBusch force-pushed the fb-serial-executor-performance branch from d18277f to a945ffd Compare October 9, 2023 12:33
@FranzBusch FranzBusch enabled auto-merge (squash) October 9, 2023 12:35
@FranzBusch FranzBusch merged commit c2d8eed into apple:main Oct 9, 2023
8 checks passed
@FranzBusch FranzBusch deleted the fb-serial-executor-performance branch October 9, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants