swift-nio icon indicating copy to clipboard operation
swift-nio copied to clipboard

Xcode 14 Thread Performance Checker Warning

Open Sherlouk opened this issue 1 year ago • 4 comments

Expected behavior

When I create a new MultiThreadedEventLoopGroup, I should not receive a performance warning from Xcode.

Actual behavior

When I create a new MultiThreadedEventLoopGroup, Xcode's new Thread Performance Checker throws the warning below. It does not appear to impede functionality. The thread checker can be turned off to silence this warning but I feel an issue here is best served to look to address the potential underlying issue.

Thread Performance Checker: Thread running at QOS_CLASS_USER_INTERACTIVE waiting on a lower QoS thread running at QOS_CLASS_DEFAULT. Investigate ways to avoid priority inversions
Full Backtrace
Thread Performance Checker: Thread running at QOS_CLASS_USER_INTERACTIVE waiting on a lower QoS thread running at QOS_CLASS_DEFAULT. Investigate ways to avoid priority inversions
PID: 71975, TID: 24582681
Backtrace
=================================================================
3   Application                           0x0000000104b9df5a $s8NIOPosix27MultiThreadedEventLoopGroupC014setupThreadAnddE033_C2B1528F4FBA68A3DBFA89DBAEBE9D4DLL4name06parentF015selectorFactory11initializerAA010SelectabledE0CSS_AcA8SelectorCyAA15NIORegistrationVGyKcyAA9NIOThreadCctFZ + 586
4   Application                           0x0000000104b9ecba $s8NIOPosix27MultiThreadedEventLoopGroupC18threadInitializers15selectorFactoryACSayyAA9NIOThreadCcG_AA8SelectorCyAA15NIORegistrationVGyKctcfcAA010SelectabledE0CyAGcXEfU_ + 650
5   Application                           0x0000000104b9ed47 $s8NIOPosix27MultiThreadedEventLoopGroupC18threadInitializers15selectorFactoryACSayyAA9NIOThreadCcG_AA8SelectorCyAA15NIORegistrationVGyKctcfcAA010SelectabledE0CyAGcXEfU_TA + 39
6   Application                           0x0000000104b9edbe $s8NIOPosix9NIOThreadCIegg_AA19SelectableEventLoopCs5Error_pIggozo_xq_r0_lyACytIsegnr_AEsAF_pIegnrzo_TR + 110
7   Application                           0x0000000104b9ee34 $s8NIOPosix9NIOThreadCIegg_AA19SelectableEventLoopCs5Error_pIggozo_xq_r0_lyACytIsegnr_AEsAF_pIegnrzo_TRTA + 20
8   libswiftCore.dylib                    0x00007ff80d64c98e $sSlsE3mapySayqd__Gqd__7ElementQzKXEKlF + 462
9   Application                           0x0000000104b9e972 $s8NIOPosix27MultiThreadedEventLoopGroupC18threadInitializers15selectorFactoryACSayyAA9NIOThreadCcG_AA8SelectorCyAA15NIORegistrationVGyKctcfc + 546
10  Application                           0x0000000104b9e744 $s8NIOPosix27MultiThreadedEventLoopGroupC18threadInitializers15selectorFactoryACSayyAA9NIOThreadCcG_AA8SelectorCyAA15NIORegistrationVGyKctcfC + 68
11  Application                           0x0000000104b9e615 $s8NIOPosix27MultiThreadedEventLoopGroupC15numberOfThreads15selectorFactoryACSi_AA8SelectorCyAA15NIORegistrationVGyKctcfC + 325
12  Application                           0x0000000104b9e43c $s8NIOPosix27MultiThreadedEventLoopGroupC15numberOfThreadsACSi_tcfC + 44
13  Application                           0x0000000104552e81 $s9Application4OTLPC5setupyyF + 609
14  Application                           0x0000000104579b0a $s9Application0A3AppVACycfC + 106
15  Application                           0x000000010457a1e9 $s9Application0A3AppV7SwiftUI0B0AadEPxycfCTW + 9
16  SwiftUI                               0x000000010cf918f2 __swift_memcpy195_8 + 10720
17  Application                           0x000000010457a17e $s9Application0A3AppV5$mainyyFZ + 30
18  Application                           0x000000010457ad59 main + 9

If possible, minimal yet complete reproducer code (or URL to code)

I have a project which pulls in tag 1.1.4 of https://github.com/open-telemetry/opentelemetry-swift into a fresh iOS project. I have added simply this line let _ = MultiThreadedEventLoopGroup(numberOfThreads: 1) to the project. When I run the project, it throws the warning above.

SwiftNIO version/commit hash

tags/2.40.0

System & version information

macOS Monterey 12.4 (21F79) Xcode Version 14.0 beta 2 (14A5229c) iOS Simulator 16.0

I have snapped a nio-diagnose.md which can be shared privately on request. I'm not sure given the kind of issue we're exploring here that it's entirely necessary.

Sherlouk avatar Jul 06 '22 22:07 Sherlouk

Yeah, we've seen this issue as well. The risk here is extremely low. Specifically, the wait is in this code: https://github.com/apple/swift-nio/blob/8b160c3681b95d88a2c0b5ad825d0c13b727c1cf/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift#L98-L124

The calling thread is waiting for the construction of a background thread. There is unlikely to be a priority inversion issue here. In the short term, you can suppress it by not creating your MultiThreadedEventLoopGroup on the main thread.

Lukasa avatar Jul 07 '22 09:07 Lukasa

Thanks Lukasa, I can confirm that moving my code onto a background thread has at least silenced the warning.

Sherlouk avatar Jul 07 '22 11:07 Sherlouk

Hey @Sherlouk I am facing the same issue. Can you show me the piece of code how you moved your code in background thread as an example?

Thanks

iRiziya avatar Sep 12 '22 06:09 iRiziya

Hey @Sherlouk I am facing the same issue. Can you show me the piece of code how you moved your code in background thread as an example?

Simply the line which instantiates the event group 😊

Sherlouk avatar Sep 12 '22 12:09 Sherlouk

I had been using Xcode 14 just fine - without that warning. After I upgraded to Ventura (macOS 13), it began.

image

RobertoMachorro avatar Oct 27 '22 12:10 RobertoMachorro

Any update on this? I too facing this issue.

venkat-epifi avatar Nov 11 '22 10:11 venkat-epifi

There is no update on this

Lukasa avatar Nov 11 '22 11:11 Lukasa

/**
 * Get the gRPC channel from the credentials.
 */
func getGrpcChannel() -> GRPCChannel {
    let grpcChannel = try! GRPCChannelPool.with(
        target: .host(self.host, port: self.port),
        transportSecurity: .tls(self.defaultClientTLSConfiguration),
        eventLoopGroup: MultiThreadedEventLoopGroup.init(numberOfThreads: 1)) {
            // Configure keepalive.
            $0.keepalive = keepalive
            $0.idleTimeout = .seconds(60)
        }
    return grpcChannel
}

Here, how to initialise the MultithreadedEventLoopGroup in DispatchQueue.global(). Basically, I have to obtain the configuration and return the GRPCChannel. If I use dispatchQueue here, since the eventloopgroup is not already initialised, I cant return the value. Any suggestion on this. How to move MultiThreadedEventLoopGroup to background thread

sridharprasath94 avatar Dec 09 '22 13:12 sridharprasath94

Hey @Sherlouk I am facing the same issue. Can you show me the piece of code how you moved your code in background thread as an example?

Simply the line which instantiates the event group 😊

Could you provide us with some exact codes?

swbenjamin avatar Jan 16 '23 22:01 swbenjamin

@swbenjamin, you can do something like this:

DispatchQueue.global(qos: .background).async {
    let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
    (...)
}

This way I'm no longer receiving the Xcode warning :)

miguel-arrf avatar Mar 08 '23 14:03 miguel-arrf

@swbenjamin, you can do something like this:

DispatchQueue.global(qos: .background).async {
    let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
    (...)
}

This way I'm no longer receiving the Xcode warning :)

I tried, but the grpc's connect was slow and unstable

oyanglun000 avatar Jul 25 '23 02:07 oyanglun000

I'm also getting this error. I have the following code

private var threadGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)

func setup(credentialsManager: CredentialsManager) async throws {
    self.credentialsManager = credentialsManager
    let channel = try GRPCChannelPool.with(
        target: .host("localhost", port: 10000),
        transportSecurity: .plaintext,
        eventLoopGroup: self.threadGroup
    )
    authClient = AuthNIOClient.init(channel: channel)
    defaultClient = DefaultNIOClient.init(channel: channel)
    
    let callOptions = CallOptions(timeLimit: .timeout(.seconds(5)))
    let call = authClient!.ping(PingRequest.init(), callOptions: callOptions)
    let _ = try await call.response.get();
}

clarkmcc avatar Sep 15 '23 20:09 clarkmcc

As discussed above, the only way to confidently avoid the warning is to initialize the event loop group on a background thread.

Lukasa avatar Sep 18 '23 07:09 Lukasa

@Lukasa, I'm an experienced coder, and I'm stumped on how to resolve this. It's worth noting that many who face this challenge aren't even directly interacting with NIO; they're using Connect Swift, grpc swift, and the like.

The party responsible for introducing this issue should either offer clear, actionable guidance or address the problem correctly.

mycroftcanner avatar Sep 22 '23 11:09 mycroftcanner

@mycroftcanner if it helps, here's how I resolved this using swift gRPC

await withCheckedContinuation { continuation in
    DispatchQueue.global(qos: .background).async {
        self.threadGroup = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
        continuation.resume(returning: ())
    }
}

let channel = try GRPCChannelPool.with(
    target: .host("localhost", port: 10000),
    transportSecurity: .plaintext,
    eventLoopGroup: self.threadGroup!
)

clarkmcc avatar Sep 22 '23 14:09 clarkmcc

Thanks @clarkmcc ! But sadly I can't figure out how to specify the event loop group with connect-swift. I created an issue there https://github.com/connectrpc/connect-swift/issues/181

I don't think it should be this complicated!!! I don't understand why this issue was introduced in the first place, it was working perfectly before! @Lukasa Lukasa

mycroftcanner avatar Sep 26 '23 13:09 mycroftcanner

To be clear, nothing works less well than it did before. This warning is unavoidable with the architecture that NIO has today, unfortunately. The only solution is to change where the event loop group is initialized.

In the case of connect-swift, check the backtrace that leads to the emission of the thread performance checker warning, and then use the pattern above with a dispatch queue to push that to the background.

Lukasa avatar Sep 26 '23 15:09 Lukasa

The best solution is to use the async/await pattern as shown above.

Lukasa avatar Sep 29 '23 12:09 Lukasa

I'm seeing similar warnings with NIOThreadPool.start(). Would this be the same issue?

adam-fowler avatar Oct 05 '23 11:10 adam-fowler

Yes indeed.

Lukasa avatar Oct 05 '23 13:10 Lukasa

Is there any plan to implement a workaround in the library to suppress this warning?

The async/await pattern above is not practical for cases where we want to create a new EventLoop in a non-async constructor. It forces users of libraries that depend on swift-nio to worry about this strange quirk.

floriansegginger avatar Oct 16 '23 13:10 floriansegginger

There is no way for us to implement a non-async workaround that will suppress the warning.

The warning exists because we are synchronously waiting for asynchronous work (the creation of background threads) from a high priority thread, and that work is at lower priority. There are only four ways to fix this:

  1. Asynchronously wait from a lower priority thread. This is the proposed workaround above using continuations and background queues. Note that it is necessary that you asynchronously wait, as any synchronous wait (e.g. using semaphores or condition variables) will reintroduce the warning.

  2. Raise the priority of the background work. We could raise the priority of the NIO I/O threads to user-interactive. This is unlikely to lead to the behaviour you want, as we can starve your UI of execution time.

  3. Deprecate the synchronous constructor and replace it with an async one. This we can do, but it causes trouble for the singleton pattern, which would continue to emit the warnings. We will be adding an async constructor, but I do not think we can go so far as to deprecate the synchronous one, and this does not solve your problem.

  4. Make MTELG construction actually synchronous: that is, we do not wait for the background threads to launch before we return.

    This is the actual fix. This fix is somewhat complicated because we need to ensure that we can construct the SelectableEventLoop object completely enough to be able to use it even when the background threads are not running. The problem there is that in our current design we require access to the pthread_t in order to implement isCurrentThread (which we use to determine what event loop we're on).

    Dealing with this is reasonably possible, but it requires some careful surgery in the core of the code. Nonetheless, this fix is planned.

Lukasa avatar Oct 16 '23 15:10 Lukasa

Thanks @Lukasa for the very detailed answer.

On my side, I have implemented a workaround where I create the SelectableEventLoop in a background thread and signal a semaphore when that is done. The start() method of my server, which is already async, waits for the semaphore using a CheckedContinuation.

This doesn't really solve the problem as in the deinit function, I will still need to block (in the very rare case that my server is destructed before fully being constructed) - that is however a compromise I am willing to accept.

floriansegginger avatar Oct 16 '23 15:10 floriansegginger

@Lukasa maybe we can add an await MultiThreadedEventLoopGroup.asyncCreate(numberOfThreads: ...) (better name required) and sort it out there. Same for pool.start(). This doesn't need to be synchronous, we can additionally offer an async function.

weissi avatar Oct 23 '23 11:10 weissi

This is roughly the idea, though I think we can actually fix the problem more broadly. @rnro should be working on this sometime soon.

Lukasa avatar Oct 23 '23 12:10 Lukasa

hi all, I found a solution from the following link:

"Task (priority: .background) is needed instead of Task to wrap asynchronous code. The essential reason is that swift-nio initializes MultiThreadedEventLoopGroup."

and it works.

https://github.com/buhe/langchain-swift/issues/68

This is roughly the idea, though I think we can actually fix the problem more broadly. @rnro should be working on this sometime soon.

yarkfu avatar Nov 29 '23 04:11 yarkfu

Any updates?

mycroftcanner avatar Dec 30 '23 13:12 mycroftcanner

I have a half-written patch that might resolve this issue, but it needs a bit more polishing before it's ready to go.

Lukasa avatar Jan 02 '24 09:01 Lukasa

See #2618 for a possible solution.

Lukasa avatar Jan 02 '24 13:01 Lukasa