diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1dda12..0748872 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,7 @@ jobs: build: name: Build (${{ matrix.name }}) runs-on: ${{ matrix.runner }} + timeout-minutes: 30 strategy: fail-fast: false matrix: @@ -43,4 +44,4 @@ jobs: run: swift build - name: Test - run: swift test + run: swift test --parallel --skip Performance --skip Benchmark --skip MetalComputeTests diff --git a/Sources/JXLSwift/Core/Patch.swift b/Sources/JXLSwift/Core/Patch.swift index be18467..b77b1db 100644 --- a/Sources/JXLSwift/Core/Patch.swift +++ b/Sources/JXLSwift/Core/Patch.swift @@ -226,6 +226,11 @@ public struct PatchDetector { let expandedHeight = min(height + initialSize, maxExpansion, currentFrame.height - destY, referenceFrame.height - sourceY) + // Stop if no growth is possible in either dimension + if expandedWidth == width && expandedHeight == height { + break + } + let expandedSimilarity = computeRegionSimilarity( currentFrame: currentFrame, referenceFrame: referenceFrame, @@ -244,11 +249,6 @@ public struct PatchDetector { width = expandedWidth height = expandedHeight - - // If we can't expand further, stop - if width >= maxExpansion && height >= maxExpansion { - break - } } // Calculate final similarity for the chosen size diff --git a/Sources/JXLSwift/Encoding/VarDCTEncoder.swift b/Sources/JXLSwift/Encoding/VarDCTEncoder.swift index 8b727f8..e7513f8 100644 --- a/Sources/JXLSwift/Encoding/VarDCTEncoder.swift +++ b/Sources/JXLSwift/Encoding/VarDCTEncoder.swift @@ -709,12 +709,9 @@ class VarDCTEncoder { // Thread-safety synchronization: // - DispatchGroup: Coordinates completion of all async operations - // - resultsLock (NSLock): Protects shared mutable state (processedResults, hasError) - // - All modifications to processedResults/hasError must be within resultsLock critical sections + // - DCTBatchState (lock-protected): Protects shared mutable state (processedResults, hasError) let dispatchGroup = DispatchGroup() - var processedResults: [[Float]?] = Array(repeating: nil, count: flatBlocks.count) - let resultsLock = NSLock() - var hasError = false + let batchState = DCTBatchState(count: flatBlocks.count) // Process batches with double-buffering via async pipeline for batchStart in stride(from: 0, to: flatBlocks.count, by: Self.metalBatchSize) { @@ -739,9 +736,7 @@ class VarDCTEncoder { defer { dispatchGroup.leave() } guard let transformed = result else { - resultsLock.lock() - hasError = true - resultsLock.unlock() + batchState.setError() return } @@ -757,11 +752,7 @@ class VarDCTEncoder { } // Store results - resultsLock.lock() - for (idx, blockResult) in batchResults.enumerated() { - processedResults[batchStart + idx] = blockResult - } - resultsLock.unlock() + batchState.setResults(batchResults, startingAt: batchStart) } } @@ -769,10 +760,12 @@ class VarDCTEncoder { dispatchGroup.wait() // Check for errors - if hasError { + if batchState.hadError { return nil } + let processedResults = batchState.getResults() + // Reconstruct 4D block structure from flat results var blocks = [[[[Float]]]]( repeating: [[[Float]]]( @@ -809,6 +802,50 @@ class VarDCTEncoder { return blocks } + + /// Thread-safe state container for DCT batch processing results + /// + /// Thread Safety: Uses `@unchecked Sendable` with `NSLock` protection for + /// mutable `results` and `hasError`. All mutations go through the lock. + private final class DCTBatchState: @unchecked Sendable { + private var results: [[Float]?] + private var _hasError: Bool = false + private let lock = NSLock() + + init(count: Int) { + self.results = Array(repeating: nil, count: count) + } + + /// Mark that an error occurred + func setError() { + lock.lock() + defer { lock.unlock() } + _hasError = true + } + + /// Set batch results starting at the given index + func setResults(_ batchResults: [[Float]], startingAt offset: Int) { + lock.lock() + defer { lock.unlock() } + for (idx, blockResult) in batchResults.enumerated() { + results[offset + idx] = blockResult + } + } + + /// Whether an error occurred during processing + var hadError: Bool { + lock.lock() + defer { lock.unlock() } + return _hasError + } + + /// Get the final results array + func getResults() -> [[Float]?] { + lock.lock() + defer { lock.unlock() } + return results + } + } #endif /// Compute the CfL correlation coefficient for a single block. diff --git a/Sources/JXLSwift/Hardware/Metal/MetalCompute.swift b/Sources/JXLSwift/Hardware/Metal/MetalCompute.swift index 5c15dcc..ea34156 100644 --- a/Sources/JXLSwift/Hardware/Metal/MetalCompute.swift +++ b/Sources/JXLSwift/Hardware/Metal/MetalCompute.swift @@ -582,10 +582,7 @@ public final class MetalAsyncPipeline: @unchecked Sendable { isProcessing = true lock.unlock() - var results: [[Float]?] = Array(repeating: nil, count: batches.count) - var completedCount = 0 - let totalBatches = batches.count - let resultsLock = NSLock() + let state = BatchState(count: batches.count) // Process batches with pipelining for (index, batch) in batches.enumerated() { @@ -594,17 +591,13 @@ public final class MetalAsyncPipeline: @unchecked Sendable { width: batch.width, height: batch.height ) { result in - resultsLock.lock() - results[index] = result - completedCount += 1 - let isDone = completedCount == totalBatches - resultsLock.unlock() + let isDone = state.setResult(result, at: index) if isDone { self.lock.lock() self.isProcessing = false self.lock.unlock() - completion(results) + completion(state.getResults()) } } } @@ -616,4 +609,37 @@ public final class MetalAsyncPipeline: @unchecked Sendable { } } +/// Thread-safe state container for batch processing results +/// +/// Thread Safety: Uses `@unchecked Sendable` with `NSLock` protection for +/// mutable `results` and `completedCount`. All mutations go through the lock. +@available(macOS 13.0, iOS 16.0, tvOS 16.0, *) +private final class BatchState: @unchecked Sendable { + private var results: [[Float]?] + private var completedCount: Int = 0 + private let totalBatches: Int + private let lock = NSLock() + + init(count: Int) { + self.results = Array(repeating: nil, count: count) + self.totalBatches = count + } + + /// Set a result at the given index and return whether all batches are done + func setResult(_ result: [Float]?, at index: Int) -> Bool { + lock.lock() + defer { lock.unlock() } + results[index] = result + completedCount += 1 + return completedCount == totalBatches + } + + /// Get the final results array + func getResults() -> [[Float]?] { + lock.lock() + defer { lock.unlock() } + return results + } +} + #endif // canImport(Metal) diff --git a/Sources/JXLSwift/Hardware/Metal/MetalOps.swift b/Sources/JXLSwift/Hardware/Metal/MetalOps.swift index e26f267..352cbf0 100644 --- a/Sources/JXLSwift/Hardware/Metal/MetalOps.swift +++ b/Sources/JXLSwift/Hardware/Metal/MetalOps.swift @@ -18,7 +18,7 @@ public enum MetalOps { // MARK: - Device Management /// Shared Metal device instance (lazy-initialized) - private static var _device: MTLDevice? + private nonisolated(unsafe) static var _device: MTLDevice? private static let deviceLock = NSLock() /// Get the default Metal device @@ -47,7 +47,7 @@ public enum MetalOps { // MARK: - Command Queue Management /// Shared command queue (lazy-initialized) - private static var _commandQueue: MTLCommandQueue? + private nonisolated(unsafe) static var _commandQueue: MTLCommandQueue? private static let queueLock = NSLock() /// Get the shared command queue @@ -66,7 +66,7 @@ public enum MetalOps { // MARK: - Shader Library Management /// Shared compute pipeline library (lazy-initialized) - private static var _library: MTLLibrary? + private nonisolated(unsafe) static var _library: MTLLibrary? private static let libraryLock = NSLock() /// Load the Metal shader library @@ -86,7 +86,7 @@ public enum MetalOps { // MARK: - Pipeline State Cache /// Cache for compute pipeline states - private static var pipelineCache: [String: MTLComputePipelineState] = [:] + private nonisolated(unsafe) static var pipelineCache: [String: MTLComputePipelineState] = [:] private static let pipelineLock = NSLock() /// Get or create a compute pipeline state for the given function name