Skip to content

Commit d53797e

Browse files
committed
Removes atomic wrappers infavor of using them directly
Replacing the usage of Int32, and UInt32 with Int since the previous was only used because of c bridging Uses .sequentiallyConsistent for all loading and storing methods Address PR comments
1 parent ccf6951 commit d53797e

19 files changed

+104
-153
lines changed

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

+5-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import SKLogging
2222
@preconcurrency package import SPMBuildCore
2323
import SourceControl
2424
import SwiftExtensions
25+
import Synchronization
2526
import TSCExtensions
2627
@preconcurrency import Workspace
2728

@@ -88,7 +89,7 @@ fileprivate extension TSCBasic.AbsolutePath {
8889
}
8990
}
9091

91-
fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)
92+
fileprivate let preparationTaskID: Atomic<Int> = Atomic(0)
9293

9394
/// Swift Package Manager build system and workspace support.
9495
///
@@ -613,7 +614,9 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
613614
}
614615
let start = ContinuousClock.now
615616

616-
let taskID: TaskId = TaskId(id: "preparation-\(preparationTaskID.fetchAndIncrement())")
617+
let taskID: TaskId = TaskId(
618+
id: "preparation-\(preparationTaskID.add(1, ordering: .sequentiallyConsistent).oldValue)"
619+
)
617620
logMessageToIndexLog(
618621
taskID,
619622
"""

Sources/CompletionScoringTestSupport/TestExtensions.swift

+4-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import Foundation
1414
import SwiftExtensions
15+
import Synchronization
1516
import XCTest
1617

1718
#if compiler(>=6)
@@ -106,7 +107,7 @@ extension XCTestCase {
106107
UserDefaults.standard.string(forKey: "TestMeasurementLogPath")
107108
}()
108109

109-
static let printBeginingOfLog = AtomicBool(initialValue: true)
110+
static let printBeginingOfLog: Atomic<Bool> = Atomic(true)
110111

111112
private static func openPerformanceLog() throws -> FileHandle? {
112113
try measurementsLogFile.map { path in
@@ -119,9 +120,9 @@ extension XCTestCase {
119120
}
120121
let logFD = try FileHandle(forWritingAtPath: path).unwrap(orThrow: "Opening \(path) failed")
121122
try logFD.seekToEnd()
122-
if printBeginingOfLog.value {
123+
if printBeginingOfLog.load(ordering: .sequentiallyConsistent) {
123124
try logFD.print("========= \(Date().description(with: .current)) =========")
124-
printBeginingOfLog.value = false
125+
printBeginingOfLog.store(false, ordering: .sequentiallyConsistent)
125126
}
126127
return logFD
127128
}

Sources/Diagnose/DiagnoseCommand.swift

+8-6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import class TSCBasic.Process
3434
import class TSCUtility.PercentProgressAnimation
3535
#endif
3636

37+
import Synchronization
38+
3739
/// When diagnosis is started, a progress bar displayed on the terminal that shows how far the diagnose command has
3840
/// progressed.
3941
/// Can't be a member of `DiagnoseCommand` because then `DiagnoseCommand` is no longer codable, which it needs to be
@@ -233,8 +235,8 @@ package struct DiagnoseCommand: AsyncParsableCommand {
233235
throw GenericError("Failed to create log.txt")
234236
}
235237
let fileHandle = try FileHandle(forWritingTo: outputFileUrl)
236-
let bytesCollected = AtomicInt32(initialValue: 0)
237-
let processExited = AtomicBool(initialValue: false)
238+
let bytesCollected: Atomic<Int> = Atomic(0)
239+
let processExited: Atomic<Bool> = Atomic(false)
238240
// 50 MB is an average log size collected by sourcekit-lsp diagnose.
239241
// It's a good proxy to show some progress indication for the majority of the time.
240242
let expectedLogSize = 50_000_000
@@ -250,16 +252,16 @@ package struct DiagnoseCommand: AsyncParsableCommand {
250252
outputRedirection: .stream(
251253
stdout: { @Sendable bytes in
252254
try? fileHandle.write(contentsOf: bytes)
253-
bytesCollected.value += Int32(bytes.count)
254-
var progress = Double(bytesCollected.value) / Double(expectedLogSize)
255+
let count = bytesCollected.add(bytes.count, ordering: .sequentiallyConsistent).newValue
256+
var progress = Double(count) / Double(expectedLogSize)
255257
if progress > 1 {
256258
// The log is larger than we expected. Halt at 100%
257259
progress = 1
258260
}
259261
Task(priority: .high) {
260262
// We have launched an async task to call `reportProgress`, which means that the process might have exited
261263
// before we execute this task. To avoid overriding a more recent progress, add a guard.
262-
if !processExited.value {
264+
if !processExited.load(ordering: .sequentiallyConsistent) {
263265
await reportProgress(.collectingLogMessages(progress: progress), message: "Collecting log messages")
264266
}
265267
}
@@ -269,7 +271,7 @@ package struct DiagnoseCommand: AsyncParsableCommand {
269271
)
270272
try process.launch()
271273
try await process.waitUntilExit()
272-
processExited.value = true
274+
processExited.store(true, ordering: .sequentiallyConsistent)
273275
#endif
274276
}
275277

Sources/InProcessClient/InProcessSourceKitLSPClient.swift

+8-2
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ import SKOptions
2828
import SourceKitLSP
2929
#endif
3030

31+
import Synchronization
32+
3133
/// Launches a `SourceKitLSPServer` in-process and allows sending messages to it.
3234
public final class InProcessSourceKitLSPClient: Sendable {
3335
private let server: SourceKitLSPServer
3436

35-
private let nextRequestID = AtomicUInt32(initialValue: 0)
37+
private let nextRequestID: Atomic<Int> = Atomic(0)
3638

3739
public convenience init(
3840
toolchainPath: URL?,
@@ -100,7 +102,11 @@ public final class InProcessSourceKitLSPClient: Sendable {
100102

101103
/// Send the request to `server` and return the request result via a completion handler.
102104
public func send<R: RequestType>(_ request: R, reply: @Sendable @escaping (LSPResult<R.Response>) -> Void) {
103-
server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement())), reply: reply)
105+
server.handle(
106+
request,
107+
id: .number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue)),
108+
reply: reply
109+
)
104110
}
105111

106112
/// Send the notification to `server`.

Sources/LanguageServerProtocolExtensions/QueueBasedMessageHandler.swift

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import Foundation
1414
import LanguageServerProtocolJSONRPC
1515
import SKLogging
16+
import Synchronization
1617

1718
#if compiler(>=6)
1819
package import LanguageServerProtocol
@@ -41,7 +42,7 @@ package actor QueueBasedMessageHandlerHelper {
4142
private let cancellationMessageHandlingQueue = AsyncQueue<Serial>()
4243

4344
/// Notifications don't have an ID. This represents the next ID we can use to identify a notification.
44-
private let notificationIDForLogging = AtomicUInt32(initialValue: 1)
45+
private let notificationIDForLogging: Atomic<Int> = Atomic(1)
4546

4647
/// The requests that we are currently handling.
4748
///
@@ -103,7 +104,7 @@ package actor QueueBasedMessageHandlerHelper {
103104
// Only use the last two digits of the notification ID for the logging scope to avoid creating too many scopes.
104105
// See comment in `withLoggingScope`.
105106
// The last 2 digits should be sufficient to differentiate between multiple concurrently running notifications.
106-
let notificationID = notificationIDForLogging.fetchAndIncrement()
107+
let notificationID = notificationIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue
107108
withLoggingScope("notification-\(notificationID % 100)") {
108109
body()
109110
}

Sources/LanguageServerProtocolExtensions/RequestAndReply.swift

+6-4
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,29 @@ import LanguageServerProtocol
1818
import SwiftExtensions
1919
#endif
2020

21+
import Synchronization
22+
2123
/// A request and a callback that returns the request's reply
2224
package final class RequestAndReply<Params: RequestType>: Sendable {
2325
package let params: Params
2426
private let replyBlock: @Sendable (LSPResult<Params.Response>) -> Void
2527

2628
/// Whether a reply has been made. Every request must reply exactly once.
27-
private let replied: AtomicBool = AtomicBool(initialValue: false)
29+
private let replied: Atomic<Bool> = Atomic(false)
2830

2931
package init(_ request: Params, reply: @escaping @Sendable (LSPResult<Params.Response>) -> Void) {
3032
self.params = request
3133
self.replyBlock = reply
3234
}
3335

3436
deinit {
35-
precondition(replied.value, "request never received a reply")
37+
precondition(replied.load(ordering: .sequentiallyConsistent), "request never received a reply")
3638
}
3739

3840
/// Call the `replyBlock` with the result produced by the given closure.
3941
package func reply(_ body: @Sendable () async throws -> Params.Response) async {
40-
precondition(!replied.value, "replied to request more than once")
41-
replied.value = true
42+
precondition(!replied.load(ordering: .sequentiallyConsistent), "replied to request more than once")
43+
replied.store(true, ordering: .sequentiallyConsistent)
4244
do {
4345
replyBlock(.success(try await body()))
4446
} catch {

Sources/SKLogging/NonDarwinLogging.swift

+5-3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import Foundation
2323
@preconcurrency import Foundation
2424
#endif
2525

26+
import Synchronization
27+
2628
// MARK: - Log settings
2729

2830
package enum LogConfig {
@@ -407,14 +409,14 @@ package struct NonDarwinLogger: Sendable {
407409
// MARK: - Signposter
408410

409411
package struct NonDarwinSignpostID: Sendable {
410-
fileprivate let id: UInt32
412+
fileprivate let id: Int
411413
}
412414

413415
package struct NonDarwinSignpostIntervalState: Sendable {
414416
fileprivate let id: NonDarwinSignpostID
415417
}
416418

417-
private let nextSignpostID = AtomicUInt32(initialValue: 0)
419+
private let nextSignpostID: Atomic<Int> = Atomic(0)
418420

419421
/// A type that is API-compatible to `OSLogMessage` for all uses within sourcekit-lsp.
420422
///
@@ -427,7 +429,7 @@ package struct NonDarwinSignposter: Sendable {
427429
}
428430

429431
package func makeSignpostID() -> NonDarwinSignpostID {
430-
return NonDarwinSignpostID(id: nextSignpostID.fetchAndIncrement())
432+
return NonDarwinSignpostID(id: nextSignpostID.add(1, ordering: .sequentiallyConsistent).oldValue)
431433
}
432434

433435
package func beginInterval(

Sources/SKTestSupport/TestSourceKitLSPClient.swift

+4-3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import SKUtilities
1818
import SourceKitD
1919
import SwiftExtensions
2020
import SwiftSyntax
21+
import Synchronization
2122
import XCTest
2223

2324
#if compiler(>=6)
@@ -87,7 +88,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
8788
package typealias RequestHandler<Request: RequestType> = @Sendable (Request) -> Request.Response
8889

8990
/// The ID that should be assigned to the next request sent to the `server`.
90-
private let nextRequestID = AtomicUInt32(initialValue: 0)
91+
private let nextRequestID: Atomic<Int> = Atomic(0)
9192

9293
/// The server that handles the requests.
9394
package let server: SourceKitLSPServer
@@ -216,7 +217,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
216217
// It's really unfortunate that there are no async deinits. If we had async
217218
// deinits, we could await the sending of a ShutdownRequest.
218219
let sema = DispatchSemaphore(value: 0)
219-
server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in
220+
server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue))) { result in
220221
sema.signal()
221222
}
222223
sema.wait()
@@ -251,7 +252,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
251252
_ request: R,
252253
completionHandler: @Sendable @escaping (LSPResult<R.Response>) -> Void
253254
) -> RequestID {
254-
let requestID = RequestID.number(Int(nextRequestID.fetchAndIncrement()))
255+
let requestID = RequestID.number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue))
255256
server.handle(request, id: requestID) { result in
256257
completionHandler(result)
257258
}

Sources/SemanticIndex/IndexTaskDescription.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ protocol IndexTaskDescription: TaskDescriptionProtocol {
2020
/// different types in `AnyIndexTaskDescription`
2121
static var idPrefix: String { get }
2222

23-
var id: UInt32 { get }
23+
var id: Int { get }
2424
}
2525

2626
extension IndexTaskDescription {

Sources/SemanticIndex/PreparationTaskDescription.swift

+4-2
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,17 @@ import struct TSCBasic.AbsolutePath
3434
import class TSCBasic.Process
3535
#endif
3636

37-
private let preparationIDForLogging = AtomicUInt32(initialValue: 1)
37+
import Synchronization
38+
39+
private let preparationIDForLogging: Atomic<Int> = Atomic(1)
3840

3941
/// Describes a task to prepare a set of targets.
4042
///
4143
/// This task description can be scheduled in a `TaskScheduler`.
4244
package struct PreparationTaskDescription: IndexTaskDescription {
4345
package static let idPrefix = "prepare"
4446

45-
package let id = preparationIDForLogging.fetchAndIncrement()
47+
package let id = preparationIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue
4648

4749
/// The targets that should be prepared.
4850
package let targetsToPrepare: [BuildTargetIdentifier]

Sources/SemanticIndex/TaskScheduler.swift

+13-11
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import SKLogging
2222
import SwiftExtensions
2323
#endif
2424

25+
import Synchronization
26+
2527
/// See comment on ``TaskDescriptionProtocol/dependencies(to:taskPriority:)``
2628
package enum TaskDependencyAction<TaskDescription: TaskDescriptionProtocol> {
2729
case waitAndElevatePriorityOfDependency(TaskDescription)
@@ -133,18 +135,18 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
133135
/// Every time `execute` gets called, a new task is placed in this continuation. See comment on `executionTask`.
134136
private let executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation
135137

136-
private let _priority: AtomicUInt8
138+
private let _priority: Atomic<UInt8>
137139

138140
/// The latest known priority of the task.
139141
///
140142
/// This starts off as the priority with which the task is being created. If higher priority tasks start depending on
141143
/// it, the priority may get elevated.
142144
nonisolated var priority: TaskPriority {
143145
get {
144-
TaskPriority(rawValue: _priority.value)
146+
TaskPriority(rawValue: _priority.load(ordering: .sequentiallyConsistent))
145147
}
146148
set {
147-
_priority.value = newValue.rawValue
149+
_priority.store(newValue.rawValue, ordering: .sequentiallyConsistent)
148150
}
149151
}
150152

@@ -154,13 +156,13 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
154156
private var cancelledToBeRescheduled: Bool = false
155157

156158
/// Whether `resultTask` has been cancelled.
157-
private let resultTaskCancelled: AtomicBool = .init(initialValue: false)
159+
private let resultTaskCancelled: Atomic<Bool> = Atomic(false)
158160

159-
private let _isExecuting: AtomicBool = .init(initialValue: false)
161+
private let _isExecuting: Atomic<Bool> = Atomic(false)
160162

161163
/// Whether the task is currently executing or still queued to be executed later.
162164
package nonisolated var isExecuting: Bool {
163-
return _isExecuting.value
165+
return _isExecuting.load(ordering: .sequentiallyConsistent)
164166
}
165167

166168
package nonisolated func cancel() {
@@ -193,7 +195,7 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
193195
description: TaskDescription,
194196
executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
195197
) async {
196-
self._priority = AtomicUInt8(initialValue: priority.rawValue)
198+
self._priority = Atomic<UInt8>(priority.rawValue)
197199
self.description = description
198200
self.executionStateChangedCallback = executionStateChangedCallback
199201

@@ -225,7 +227,7 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
225227
self.priority = Task.currentPriority
226228
}
227229
} onCancel: {
228-
self.resultTaskCancelled.value = true
230+
self.resultTaskCancelled.store(true, ordering: .sequentiallyConsistent)
229231
}
230232
}
231233
}
@@ -246,12 +248,12 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
246248
}
247249
precondition(executionTask == nil, "Task started twice")
248250
let task = Task.detached(priority: self.priority) {
249-
if !Task.isCancelled && !self.resultTaskCancelled.value {
251+
if !Task.isCancelled && !self.resultTaskCancelled.load(ordering: .sequentiallyConsistent) {
250252
await self.description.execute()
251253
}
252254
return await self.finalizeExecution()
253255
}
254-
_isExecuting.value = true
256+
_isExecuting.store(true, ordering: .sequentiallyConsistent)
255257
executionTask = task
256258
executionTaskCreatedContinuation.yield(task)
257259
await executionStateChangedCallback?(self, .executing)
@@ -261,7 +263,7 @@ package actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
261263
/// Implementation detail of `execute` that is called after `self.description.execute()` finishes.
262264
private func finalizeExecution() async -> ExecutionTaskFinishStatus {
263265
self.executionTask = nil
264-
_isExecuting.value = false
266+
_isExecuting.store(false, ordering: .sequentiallyConsistent)
265267
if Task.isCancelled && self.cancelledToBeRescheduled {
266268
await executionStateChangedCallback?(self, .cancelledToBeRescheduled)
267269
self.cancelledToBeRescheduled = false

0 commit comments

Comments
 (0)