Skip to content

(chore): Use Native swift Atomics #1969

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Contributor Documentation/Modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ Swift types to represent the [Build Server Protocol (BSP) specification](https:/

Defines the queries SourceKit-LSP can ask of a build system, like getting compiler arguments for a file, finding a target’s dependencies or preparing a target.

### CAtomics

Implementation of atomics for Swift using C. Once we can raise our deployment target to use the `Atomic` type from the Swift standard library, this module should be removed.

### CSKTestSupport

For testing, overrides `__cxa_atexit` to prevent registration of static destructors due to work around https://github.com/swiftlang/swift/issues/55112.
Expand Down
11 changes: 1 addition & 10 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,6 @@ var targets: [Target] = [
swiftSettings: globalSwiftSettings
),

// MARK: CAtomics

.target(
name: "CAtomics",
dependencies: []
),

.target(
name: "CCompletionScoring",
dependencies: []
Expand Down Expand Up @@ -526,14 +519,12 @@ var targets: [Target] = [

.target(
name: "SwiftExtensions",
dependencies: ["CAtomics"],
exclude: ["CMakeLists.txt"],
swiftSettings: globalSwiftSettings
),

.target(
name: "SwiftExtensionsForPlugin",
dependencies: ["CAtomics"],
exclude: ["CMakeLists.txt"],
swiftSettings: globalSwiftSettings
),
Expand Down Expand Up @@ -694,7 +685,7 @@ if buildOnlyTests {

let package = Package(
name: "SourceKitLSP",
platforms: [.macOS(.v13)],
platforms: [.macOS("15")],
products: products,
dependencies: dependencies,
targets: targets,
Expand Down
7 changes: 5 additions & 2 deletions Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import SKLogging
@preconcurrency package import SPMBuildCore
import SourceControl
import SwiftExtensions
import Synchronization
import TSCExtensions
@preconcurrency import Workspace

Expand Down Expand Up @@ -88,7 +89,7 @@ fileprivate extension TSCBasic.AbsolutePath {
}
}

fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)
fileprivate let preparationTaskID: Atomic<Int> = Atomic(0)

/// Swift Package Manager build system and workspace support.
///
Expand Down Expand Up @@ -613,7 +614,9 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
}
let start = ContinuousClock.now

let taskID: TaskId = TaskId(id: "preparation-\(preparationTaskID.fetchAndIncrement())")
let taskID: TaskId = TaskId(
id: "preparation-\(preparationTaskID.add(1, ordering: .sequentiallyConsistent).oldValue)"
)
logMessageToIndexLog(
taskID,
"""
Expand Down
Empty file removed Sources/CAtomics/CAtomics.c
Empty file.
2 changes: 0 additions & 2 deletions Sources/CAtomics/CMakeLists.txt

This file was deleted.

73 changes: 0 additions & 73 deletions Sources/CAtomics/include/CAtomics.h

This file was deleted.

4 changes: 0 additions & 4 deletions Sources/CAtomics/include/module.modulemap

This file was deleted.

1 change: 0 additions & 1 deletion Sources/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ add_compile_options("$<$<COMPILE_LANGUAGE:Swift>:SHELL:-DRESILIENT_LIBRARIES>")
add_compile_options("$<$<COMPILE_LANGUAGE:Swift>:SHELL:-swift-version 6>")
add_subdirectory(BuildServerProtocol)
add_subdirectory(BuildSystemIntegration)
add_subdirectory(CAtomics)
add_subdirectory(CCompletionScoring)
add_subdirectory(CompletionScoring)
add_subdirectory(Csourcekitd)
Expand Down
7 changes: 4 additions & 3 deletions Sources/CompletionScoringTestSupport/TestExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import Foundation
import SwiftExtensions
import Synchronization
import XCTest

#if compiler(>=6)
Expand Down Expand Up @@ -106,7 +107,7 @@ extension XCTestCase {
UserDefaults.standard.string(forKey: "TestMeasurementLogPath")
}()

static let printBeginingOfLog = AtomicBool(initialValue: true)
static let printBeginingOfLog: Atomic<Bool> = Atomic(true)

private static func openPerformanceLog() throws -> FileHandle? {
try measurementsLogFile.map { path in
Expand All @@ -119,9 +120,9 @@ extension XCTestCase {
}
let logFD = try FileHandle(forWritingAtPath: path).unwrap(orThrow: "Opening \(path) failed")
try logFD.seekToEnd()
if printBeginingOfLog.value {
if printBeginingOfLog.load(ordering: .sequentiallyConsistent) {
try logFD.print("========= \(Date().description(with: .current)) =========")
printBeginingOfLog.value = false
printBeginingOfLog.store(false, ordering: .sequentiallyConsistent)
}
return logFD
}
Expand Down
14 changes: 8 additions & 6 deletions Sources/Diagnose/DiagnoseCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

import Synchronization

#if compiler(>=6)
package import ArgumentParser
import Foundation
Expand Down Expand Up @@ -233,8 +235,8 @@ package struct DiagnoseCommand: AsyncParsableCommand {
throw GenericError("Failed to create log.txt")
}
let fileHandle = try FileHandle(forWritingTo: outputFileUrl)
let bytesCollected = AtomicInt32(initialValue: 0)
let processExited = AtomicBool(initialValue: false)
let bytesCollected: Atomic<Int> = Atomic(0)
let processExited: Atomic<Bool> = Atomic(false)
// 50 MB is an average log size collected by sourcekit-lsp diagnose.
// It's a good proxy to show some progress indication for the majority of the time.
let expectedLogSize = 50_000_000
Expand All @@ -250,16 +252,16 @@ package struct DiagnoseCommand: AsyncParsableCommand {
outputRedirection: .stream(
stdout: { @Sendable bytes in
try? fileHandle.write(contentsOf: bytes)
bytesCollected.value += Int32(bytes.count)
var progress = Double(bytesCollected.value) / Double(expectedLogSize)
let count = bytesCollected.add(bytes.count, ordering: .sequentiallyConsistent).newValue
var progress = Double(count) / Double(expectedLogSize)
if progress > 1 {
// The log is larger than we expected. Halt at 100%
progress = 1
}
Task(priority: .high) {
// We have launched an async task to call `reportProgress`, which means that the process might have exited
// before we execute this task. To avoid overriding a more recent progress, add a guard.
if !processExited.value {
if !processExited.load(ordering: .sequentiallyConsistent) {
await reportProgress(.collectingLogMessages(progress: progress), message: "Collecting log messages")
}
}
Expand All @@ -269,7 +271,7 @@ package struct DiagnoseCommand: AsyncParsableCommand {
)
try process.launch()
try await process.waitUntilExit()
processExited.value = true
processExited.store(true, ordering: .sequentiallyConsistent)
#endif
}

Expand Down
9 changes: 7 additions & 2 deletions Sources/InProcessClient/InProcessSourceKitLSPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public import Foundation
public import LanguageServerProtocol
import LanguageServerProtocolExtensions
import SwiftExtensions
import Synchronization
import TSCExtensions

import struct TSCBasic.AbsolutePath
Expand All @@ -33,7 +34,7 @@ import ToolchainRegistry
public final class InProcessSourceKitLSPClient: Sendable {
private let server: SourceKitLSPServer

private let nextRequestID = AtomicUInt32(initialValue: 0)
private let nextRequestID: Atomic<Int> = Atomic(0)

public convenience init(
toolchainPath: URL?,
Expand Down Expand Up @@ -101,7 +102,11 @@ public final class InProcessSourceKitLSPClient: Sendable {

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

/// Send the notification to `server`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import Foundation
import LanguageServerProtocolJSONRPC
import SKLogging
import Synchronization

#if compiler(>=6)
package import LanguageServerProtocol
Expand Down Expand Up @@ -41,7 +42,7 @@ package actor QueueBasedMessageHandlerHelper {
private let cancellationMessageHandlingQueue = AsyncQueue<Serial>()

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

/// The requests that we are currently handling.
///
Expand Down Expand Up @@ -103,7 +104,7 @@ package actor QueueBasedMessageHandlerHelper {
// Only use the last two digits of the notification ID for the logging scope to avoid creating too many scopes.
// See comment in `withLoggingScope`.
// The last 2 digits should be sufficient to differentiate between multiple concurrently running notifications.
let notificationID = notificationIDForLogging.fetchAndIncrement()
let notificationID = notificationIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue
withLoggingScope("notification-\(notificationID % 100)") {
body()
}
Expand Down
10 changes: 6 additions & 4 deletions Sources/LanguageServerProtocolExtensions/RequestAndReply.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

import Synchronization

#if compiler(>=6)
package import LanguageServerProtocol
import SwiftExtensions
Expand All @@ -24,21 +26,21 @@ package final class RequestAndReply<Params: RequestType>: Sendable {
private let replyBlock: @Sendable (LSPResult<Params.Response>) -> Void

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

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

deinit {
precondition(replied.value, "request never received a reply")
precondition(replied.load(ordering: .sequentiallyConsistent), "request never received a reply")
}

/// Call the `replyBlock` with the result produced by the given closure.
package func reply(_ body: @Sendable () async throws -> Params.Response) async {
precondition(!replied.value, "replied to request more than once")
replied.value = true
precondition(!replied.load(ordering: .sequentiallyConsistent), "replied to request more than once")
replied.store(true, ordering: .sequentiallyConsistent)
do {
replyBlock(.success(try await body()))
} catch {
Expand Down
8 changes: 5 additions & 3 deletions Sources/SKLogging/NonDarwinLogging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

import Synchronization

#if compiler(>=6)
package import SwiftExtensions
#else
Expand Down Expand Up @@ -407,14 +409,14 @@ package struct NonDarwinLogger: Sendable {
// MARK: - Signposter

package struct NonDarwinSignpostID: Sendable {
fileprivate let id: UInt32
fileprivate let id: Int
}

package struct NonDarwinSignpostIntervalState: Sendable {
fileprivate let id: NonDarwinSignpostID
}

private let nextSignpostID = AtomicUInt32(initialValue: 0)
private let nextSignpostID: Atomic<Int> = Atomic(0)

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

package func makeSignpostID() -> NonDarwinSignpostID {
return NonDarwinSignpostID(id: nextSignpostID.fetchAndIncrement())
return NonDarwinSignpostID(id: nextSignpostID.add(1, ordering: .sequentiallyConsistent).oldValue)
}

package func beginInterval(
Expand Down
Loading