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 TaskExecutor conformance to EventLoops #2732

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fabianfett
Copy link
Member

@fabianfett fabianfett commented May 29, 2024

Add TaskExecutor conformance to EventLoops

  • Support NIO Event Loops as task executors

Motivation

Swift 6 introduces the option to supply your own task executors to the Swift concurrency runtime (SE-0417). SwiftNIO's EventLoop should conform to the new TaskExecutor protocol to support running Swift concurrency Tasks on it. This will allow fewer context switches in Swift on server applications.

Modifications

  • Extend EventLoop to expose an taskExecutor property (the good executor name is sadly already taken by the SerialExecutor)
  • Rename NIODefaultSerialEventLoopExecutor to NIODefaultEventLoopExecutor
  • NIODefaultEventLoopExecutor now also implements TaskExecutor protocol
  • Expose NIOTaskEventLoopExecutor protocol that makes it easy for adopters to get a better implementation
  • Add test

Result

Adopters can use ELs as task executors

@fabianfett fabianfett marked this pull request as draft May 29, 2024 12:45
@fabianfett fabianfett force-pushed the ff-nio-task-executor branch 2 times, most recently from c48fcba to a982359 Compare May 29, 2024 13:18
// MARK: TaskExecutor conformance
#if compiler(>=6.0)
@available(macOS 9999.0, iOS 9999.0, watchOS 9999.0, tvOS 9999.0, *)
extension SelectableEventLoop: NIOTaskEventLoopExecutor { }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add conformance for AsyncTestingEventLoop and a fatalErroring conformance for EmbeddedEventLoop?

let loop1 = iterator.next()!
let loop2 = iterator.next()!

await self._runTests(loop1: loop1, loop2: loop2)
Copy link
Member Author

@fabianfett fabianfett May 29, 2024

Choose a reason for hiding this comment

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

this test currently crashes:

Thread 4 Crashed:: NIO-ELT-0-#0
0   libswiftCore.dylib            	       0x19a319644 swift_getObjectType + 88
1   libswift_Concurrency.dylib    	       0x2617ea0e4 swift_task_enqueueImpl(swift::Job*, swift::SerialExecutorRef) + 160
2   libswift_Concurrency.dylib    	       0x2617ec890 swift::AsyncTask::flagAsAndEnqueueOnExecutor(swift::SerialExecutorRef) + 432
3   libswift_Concurrency.dylib    	       0x2617eacdc swift_task_switchImpl(swift::AsyncContext*, void (swift::AsyncContext* swift_async_context) swiftasynccall*, swift::SerialExecutorRef) + 640
4   swift-nioPackageTests         	       0x1080d5ca1 partial apply for closure #1 in closure #1 in TaskExecutorTests._runTests<A, B>(loop1:loop2:) + 1
5   swift-nioPackageTests         	       0x1080d4d3d thunk for @escaping @isolated(any) @callee_guaranteed @Sendable @async () -> (@out A) + 1
6   swift-nioPackageTests         	       0x1080d4e99 partial apply for thunk for @escaping @isolated(any) @callee_guaranteed @Sendable @async () -> (@out A) + 1
7   libswift_Concurrency.dylib    	       0x2617f1921 completeTaskWithClosure(swift::AsyncContext*, swift::SwiftError*) + 1

Why do we call swift_task_switchImpl with a SerialExecutorRef?

Copy link
Member

Choose a reason for hiding this comment

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

switch is what checks if it can "switch" without releasing actor locks; it won't be able to switch when there's a task executor. I'll look into reproducing this though with a debug runtime to check.

@FranzBusch
Copy link
Member

Can we also add another benchmark for the NIOAsyncChannel that uses the task executor next to the global executor hook to prove that this indeed does what we want it to do.

@fabianfett
Copy link
Member Author

fabianfett commented Jun 4, 2024

Performance results on my machine:

==================
NIOPosixBenchmarks
==================

TCPEcho pure NIO 1M times
╒═════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                  │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Context switches        │    4370 │    4370 │    4370 │    4370 │    4370 │    4370 │    4370 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) *        │     428 │     428 │     428 │     428 │     428 │     428 │     428 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ms) * │    4537 │    4537 │    4537 │    4537 │    4537 │    4537 │    4537 │       1 │
╘═════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

TCPEchoAsyncChannel pure async/await 1M times
╒═════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                  │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Context switches (K)    │     148 │     148 │     148 │     148 │     148 │     148 │     148 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) (K) *    │    1492 │    1492 │    1492 │    1492 │    1492 │    1492 │    1492 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ms) * │    8431 │    8431 │    8431 │    8431 │    8431 │    8431 │    8431 │       1 │
╘═════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

TCPEchoAsyncChannel using globalHook 1M times
╒═════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                  │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Context switches        │    9498 │    9498 │    9498 │    9498 │    9498 │    9498 │    9498 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) (K) *    │     221 │     221 │     221 │     221 │     221 │     221 │     221 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ms) * │    9011 │    9011 │    9011 │    9011 │    9011 │    9011 │    9011 │       1 │
╘═════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

TCPEchoAsyncChannel using task executor preference 1M times
╒═════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                  │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Context switches        │    8352 │    8352 │    8352 │    8352 │    8352 │    8352 │    8352 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) (K) *    │    6175 │    6175 │    6175 │    6175 │    6175 │    6175 │    6175 │       1 │
├─────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ms) * │    9336 │    9336 │    9336 │    9336 │    9336 │    9336 │    9336 │       1 │
╘═════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

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.

4 participants