Skip to content

Commit

Permalink
Remove DispatchGroup and replace with condvar (#2687)
Browse files Browse the repository at this point in the history
Motivation:

DispatchGroup can cause thread performance checker warnings in
places we don't want them. We can avoid those warnings in some
cases by using a condition variable instead.

Modifications:

- Replaced a use of DispatchGroup with a condition variable
    in NIOThreadPool setup.
- Removed an unnecessary DispatchGroup in the shutdown code

Result:

Fewer thread performance checker warnings.
  • Loading branch information
Lukasa committed Mar 20, 2024
1 parent 1e2b3e3 commit f8c5e02
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions Sources/NIOPosix/NIOThreadPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public final class NIOThreadPool {
}
return
}
let g = DispatchGroup()

let threadsToJoin = self.lock.withLock { () -> [NIOThread] in
switch self.state {
case .running(let items):
Expand All @@ -117,12 +117,11 @@ public final class NIOThreadPool {
}
}

DispatchQueue(label: "io.swiftnio.NIOThreadPool.shutdownGracefully").async(group: g) {
DispatchQueue(label: "io.swiftnio.NIOThreadPool.shutdownGracefully").async {
threadsToJoin.forEach { $0.join() }
}

g.notify(queue: queue) {
callback(nil)
queue.async {
callback(nil)
}
}
}

Expand Down Expand Up @@ -233,7 +232,10 @@ public final class NIOThreadPool {
return
}

let group = DispatchGroup()
// We use this condition lock as a tricky kind of semaphore.
// This is done to sidestep the thread performance checker warning
// that would otherwise be emitted.
let cond = ConditionLock(value: 0)

self.lock.withLock {
assert(self.threads == nil)
Expand All @@ -242,19 +244,21 @@ public final class NIOThreadPool {
}

for id in 0..<self.numberOfThreads {
group.enter()
// We should keep thread names under 16 characters because Linux doesn't allow more.
NIOThread.spawnAndRun(name: "\(threadNamePrefix)\(id)", detachThread: false) { thread in
self.lock.withLock {
self.threads!.append(thread)
cond.lock()
cond.unlock(withValue: self.threads!.count)
}
group.leave()

self.process(identifier: id)
return ()
}
}

group.wait()
cond.lock(whenValue: self.numberOfThreads)
cond.unlock()
assert(self.lock.withLock { self.threads?.count ?? -1 } == self.numberOfThreads)
}

Expand Down

0 comments on commit f8c5e02

Please sign in to comment.