From a497aadd8bccc26be0dddf1b9fae38af6a667d54 Mon Sep 17 00:00:00 2001 From: gnavadev Date: Sun, 26 Jul 2026 17:06:35 -0700 Subject: [PATCH 1/2] Fix O(n2) log tailing, cache compiled globs, and narrow disk-usage lock scope. --- Sources/ContainerBuild/BuildFSSync.swift | 35 ++--- Sources/ContainerBuild/Globber.swift | 16 ++- .../Container/ContainerLogs.swift | 87 +----------- .../Container/ContainerStats.swift | 68 ++++++---- Sources/ContainerCommands/LogTailer.swift | 127 ++++++++++++++++++ .../Machine/MachineLogs.swift | 87 +----------- .../Server/Containers/ContainersService.swift | 48 ++++--- .../Server/Volumes/VolumesService.swift | 42 ++++-- .../LogTailerTests.swift | 97 +++++++++++++ 9 files changed, 353 insertions(+), 254 deletions(-) create mode 100644 Sources/ContainerCommands/LogTailer.swift create mode 100644 Tests/ContainerCommandsTests/LogTailerTests.swift diff --git a/Sources/ContainerBuild/BuildFSSync.swift b/Sources/ContainerBuild/BuildFSSync.swift index 7b4fc4400..8e2cf4970 100644 --- a/Sources/ContainerBuild/BuildFSSync.swift +++ b/Sources/ContainerBuild/BuildFSSync.swift @@ -113,20 +113,13 @@ actor BuildFSSync: BuildPipelineHandler { sender.yield(response) } - private struct DirEntry: Hashable { - let url: URL + private struct DirEntry { let isDirectory: Bool let relativePath: String - - func hash(into hasher: inout Hasher) { - hasher.combine(relativePath) - } - - static func == (lhs: DirEntry, rhs: DirEntry) -> Bool { - lhs.relativePath == rhs.relativePath - } } + private typealias DirEntries = [String: [String: DirEntry]] + func walk( _ sender: AsyncStream.Continuation, _ packet: BuildTransfer, @@ -134,7 +127,7 @@ actor BuildFSSync: BuildPipelineHandler { ) async throws { let wantsTar = packet.mode() == "tar" - var entries: [String: Set] = [:] + var entries: DirEntries = [:] let followPaths: [String] = packet.followPaths() ?? [] let followPathsWalked = try walk(root: self.contextDir, includePatterns: followPaths) @@ -148,16 +141,14 @@ actor BuildFSSync: BuildPipelineHandler { let relPath = try url.relativeChildPath(to: contextDir) let parentPath = try url.deletingLastPathComponent().relativeChildPath(to: contextDir) - let entry = DirEntry(url: url, isDirectory: url.hasDirectoryPath, relativePath: relPath) - entries[parentPath, default: []].insert(entry) + entries[parentPath, default: [:]][relPath] = DirEntry(isDirectory: url.hasDirectoryPath, relativePath: relPath) if url.isSymlink { let target: URL = url.resolvingSymlinksInPath() if self.contextDir.parentOf(target) { let relPath = try target.relativeChildPath(to: self.contextDir) - let entry = DirEntry(url: target, isDirectory: target.hasDirectoryPath, relativePath: relPath) let parentPath: String = try target.deletingLastPathComponent().relativeChildPath(to: self.contextDir) - entries[parentPath, default: []].insert(entry) + entries[parentPath, default: [:]][relPath] = DirEntry(isDirectory: target.hasDirectoryPath, relativePath: relPath) } } } @@ -213,15 +204,7 @@ actor BuildFSSync: BuildPipelineHandler { return nil } - guard let items = entries[parent] else { - return nil - } - - let include = items.contains { item in - item.relativePath == rel - } - - guard include else { + guard entries[parent]?[rel] != nil else { return nil } @@ -300,7 +283,7 @@ actor BuildFSSync: BuildPipelineHandler { private func processDirectory( _ currentDir: String, - inputEntries: [String: Set], + inputEntries: DirEntries, processedPaths: inout [String] ) throws { guard let entries = inputEntries[currentDir] else { @@ -308,7 +291,7 @@ actor BuildFSSync: BuildPipelineHandler { } // Sort purely by lexicographical order of relativePath - let sortedEntries = entries.sorted { $0.relativePath < $1.relativePath } + let sortedEntries = entries.values.sorted { $0.relativePath < $1.relativePath } for entry in sortedEntries { processedPaths.append(entry.relativePath) diff --git a/Sources/ContainerBuild/Globber.swift b/Sources/ContainerBuild/Globber.swift index baecf60d0..fa35a0745 100644 --- a/Sources/ContainerBuild/Globber.swift +++ b/Sources/ContainerBuild/Globber.swift @@ -20,6 +20,8 @@ public class Globber { let input: URL var results: Set = .init() + private var compiledGlobs: [String: NSRegularExpression] = [:] + public init(_ input: URL) { self.input = input } @@ -76,6 +78,16 @@ public class Globber { } func glob(_ input: String, _ pattern: String) throws -> Bool { + let regex = try compiledGlob(for: pattern) + let range = NSRange(input.startIndex..., in: input) + return regex.firstMatch(in: input, options: [], range: range) != nil + } + + private func compiledGlob(for pattern: String) throws -> NSRegularExpression { + if let cached = compiledGlobs[pattern] { + return cached + } + let regexPattern = "^" + NSRegularExpression.escapedPattern(for: pattern) @@ -87,7 +99,9 @@ public class Globber { // validate the regex pattern created let _ = try Regex(regexPattern) - return input.range(of: regexPattern, options: .regularExpression) != nil + let regex = try NSRegularExpression(pattern: regexPattern) + compiledGlobs[pattern] = regex + return regex } } diff --git a/Sources/ContainerCommands/Container/ContainerLogs.swift b/Sources/ContainerCommands/Container/ContainerLogs.swift index e0eb3511e..31d833c7a 100644 --- a/Sources/ContainerCommands/Container/ContainerLogs.swift +++ b/Sources/ContainerCommands/Container/ContainerLogs.swift @@ -16,9 +16,6 @@ import ArgumentParser import ContainerAPIClient -import ContainerizationError -import Darwin -import Dispatch import Foundation extension Application { @@ -50,93 +47,11 @@ extension Application { let fhs = try await client.logs(id: containerId) let fileHandle = boot ? fhs[1] : fhs[0] - try await Self.tail( + try await LogTailer.tail( fh: fileHandle, n: numLines, follow: follow ) } - - private static func tail( - fh: FileHandle, - n: Int?, - follow: Bool - ) async throws { - if let n { - var buffer = Data() - let size = try fh.seekToEnd() - var offset = size - var lines: [String] = [] - - while offset > 0, lines.count < n { - let readSize = min(1024, offset) - offset -= readSize - try fh.seek(toOffset: offset) - - let data = fh.readData(ofLength: Int(readSize)) - buffer.insert(contentsOf: data, at: 0) - - if let chunk = String(data: buffer, encoding: .utf8) { - lines = chunk.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - } - } - - lines = Array(lines.suffix(n)) - for line in lines { - print(line) - } - } else { - // Fast path if all they want is the full file. - guard let data = try fh.readToEnd() else { - // Seems you get nil if it's a zero byte read, or you - // try and read from dev/null. - return - } - guard let str = String(data: data, encoding: .utf8) else { - throw ContainerizationError( - .internalError, - message: "failed to convert container logs to utf8" - ) - } - print(str.trimmingCharacters(in: .newlines)) - } - - fflush(stdout) - if follow { - setbuf(stdout, nil) - try await Self.followFile(fh: fh) - } - } - - private static func followFile(fh: FileHandle) async throws { - _ = try fh.seekToEnd() - let stream = AsyncStream { cont in - fh.readabilityHandler = { handle in - let data = handle.availableData - if data.isEmpty { - // Triggers on container restart - can exit here as well - do { - _ = try fh.seekToEnd() // To continue streaming existing truncated log files - } catch { - fh.readabilityHandler = nil - cont.finish() - return - } - } - if let str = String(data: data, encoding: .utf8), !str.isEmpty { - var lines = str.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - for line in lines { - cont.yield(line) - } - } - } - } - - for await line in stream { - print(line) - } - } } } diff --git a/Sources/ContainerCommands/Container/ContainerStats.swift b/Sources/ContainerCommands/Container/ContainerStats.swift index 769a28809..e75ca8fa1 100644 --- a/Sources/ContainerCommands/Container/ContainerStats.swift +++ b/Sources/ContainerCommands/Container/ContainerStats.swift @@ -160,43 +160,59 @@ extension Application { } private static func collectStats(client: ContainerClient, for containers: [ContainerSnapshot]) async throws -> [StatsSnapshot] { - var snapshots: [StatsSnapshot] = [] + let running = containers.filter { $0.status == .running } - // First sample - for container in containers { - guard container.status == .running else { continue } - do { - let stats1 = try await client.stats(id: container.id) - snapshots.append(StatsSnapshot(container: container, stats1: stats1, stats2: stats1)) - } catch { - // Skip containers that error out - continue + // First sample. Containers that error out are skipped. + let firstSample = await sample(client: client, ids: running.map { $0.id }) + var snapshots = running.compactMap { container -> StatsSnapshot? in + guard let stats = firstSample[container.id] else { + return nil } + return StatsSnapshot(container: container, stats1: stats, stats2: stats) } - // Wait 2 seconds for CPU delta calculation - if !snapshots.isEmpty { - try await Task.sleep(for: .seconds(2)) + guard !snapshots.isEmpty else { + return snapshots + } - // Second sample - for i in 0.. StatsSnapshot in + guard let stats2 = secondSample[snapshot.container.id] else { + // Keep the original stats if second sample fails + return snapshot } + return StatsSnapshot(container: snapshot.container, stats1: snapshot.stats1, stats2: stats2) } return snapshots } + /// Fetch stats for every container concurrently, omitting the ones whose + /// stats could not be read. One round trip per container in series adds + /// up quickly once more than a handful of containers are running. + private static func sample(client: ContainerClient, ids: [String]) async -> [String: ContainerResource.ContainerStats] { + await withTaskGroup( + of: (String, ContainerResource.ContainerStats?).self, + returning: [String: ContainerResource.ContainerStats].self + ) { group in + for id in ids { + group.addTask { + (id, try? await client.stats(id: id)) + } + } + + var results = [String: ContainerResource.ContainerStats]() + for await (id, stats) in group { + results[id] = stats + } + return results + } + } + /// Calculate CPU percentage from two stat snapshots /// - Parameters: /// - cpuUsageUsec1: CPU usage in microseconds from first sample diff --git a/Sources/ContainerCommands/LogTailer.swift b/Sources/ContainerCommands/LogTailer.swift new file mode 100644 index 000000000..a603dd406 --- /dev/null +++ b/Sources/ContainerCommands/LogTailer.swift @@ -0,0 +1,127 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerizationError +import Darwin +import Foundation + +enum LogTailer { + private static let chunkSize: UInt64 = 1024 + private static let newline = UInt8(ascii: "\n") + + static func tail(fh: FileHandle, n: Int?, follow: Bool) async throws { + if let n { + for line in try lastLines(fh: fh, n: n) { + print(line) + } + } else { + guard let data = try fh.readToEnd() else { + return + } + guard let str = String(data: data, encoding: .utf8) else { + throw ContainerizationError( + .internalError, + message: "failed to convert container logs to utf8" + ) + } + print(str.trimmingCharacters(in: .newlines)) + } + + fflush(stdout) + if follow { + setbuf(stdout, nil) + try await followFile(fh: fh) + } + } + + static func lastLines(fh: FileHandle, n: Int) throws -> [String] { + guard n > 0 else { + return [] + } + + var offset = try fh.seekToEnd() + var chunks: [Data] = [] + var lineCount = 0 + + while offset > 0, lineCount <= n { + let readSize = min(Self.chunkSize, offset) + offset -= readSize + try fh.seek(toOffset: offset) + + let chunk = fh.readData(ofLength: Int(readSize)) + lineCount += Self.countLines(in: chunk, precedingChunk: chunks.last) + chunks.append(chunk) + } + + var buffer = Data() + for chunk in chunks.reversed() { + buffer.append(chunk) + } + + let decodable = buffer.drop { $0 & 0xC0 == 0x80 } + guard let text = String(data: decodable, encoding: .utf8) else { + return [] + } + + let lines = text.components(separatedBy: .newlines).filter { !$0.isEmpty } + return Array(lines.suffix(n)) + } + + private static func countLines(in chunk: Data, precedingChunk: Data?) -> Int { + var count = 0 + var previous: UInt8? + for byte in chunk { + if byte != Self.newline, previous == nil || previous == Self.newline { + count += 1 + } + previous = byte + } + + if let last = chunk.last, last != Self.newline, let first = precedingChunk?.first, first != Self.newline { + count -= 1 + } + return count + } + + private static func followFile(fh: FileHandle) async throws { + _ = try fh.seekToEnd() + let stream = AsyncStream { cont in + fh.readabilityHandler = { handle in + let data = handle.availableData + if data.isEmpty { + do { + _ = try fh.seekToEnd() // To continue streaming existing truncated log files + } catch { + fh.readabilityHandler = nil + cont.finish() + return + } + } + if let str = String(data: data, encoding: .utf8), !str.isEmpty { + var lines = str.components(separatedBy: .newlines) + lines = lines.filter { !$0.isEmpty } + for line in lines { + cont.yield(line) + } + } + } + } + + for await line in stream { + print(line) + } + } +} diff --git a/Sources/ContainerCommands/Machine/MachineLogs.swift b/Sources/ContainerCommands/Machine/MachineLogs.swift index b76cebe9b..930e3d327 100644 --- a/Sources/ContainerCommands/Machine/MachineLogs.swift +++ b/Sources/ContainerCommands/Machine/MachineLogs.swift @@ -16,7 +16,6 @@ import ArgumentParser import ContainerAPIClient -import ContainerizationError import ContainerizationOS import Foundation import MachineAPIClient @@ -60,93 +59,11 @@ extension Application { let fhs = try await client.logs(id: id) let fileHandle = boot ? fhs[1] : fhs[0] - try await Self.tail( + try await LogTailer.tail( fh: fileHandle, n: numLines, - follow: follow, + follow: follow ) } - - private static func tail( - fh: FileHandle, - n: Int?, - follow: Bool - ) async throws { - if let n { - var buffer = Data() - let size = try fh.seekToEnd() - var offset = size - var lines: [String] = [] - - while offset > 0, lines.count < n { - let readSize = min(1024, offset) - offset -= readSize - try fh.seek(toOffset: offset) - - let data = fh.readData(ofLength: Int(readSize)) - buffer.insert(contentsOf: data, at: 0) - - if let chunk = String(data: buffer, encoding: .utf8) { - lines = chunk.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - } - } - - lines = Array(lines.suffix(n)) - for line in lines { - print(line) - } - } else { - // Fast path if all they want is the full file. - guard let data = try fh.readToEnd() else { - // Seems you get nil if it's a zero byte read, or you - // try and read from dev/null. - return - } - guard let str = String(data: data, encoding: .utf8) else { - throw ContainerizationError( - .internalError, - message: "failed to convert container logs to utf8" - ) - } - print(str.trimmingCharacters(in: .newlines)) - } - - fflush(stdout) - if follow { - setbuf(stdout, nil) - try await Self.followFile(fh: fh) - } - } - - private static func followFile(fh: FileHandle) async throws { - _ = try fh.seekToEnd() - let stream = AsyncStream { cont in - fh.readabilityHandler = { handle in - let data = handle.availableData - if data.isEmpty { - // Triggers on container restart - can exit here as well - do { - _ = try fh.seekToEnd() // To continue streaming existing truncated log files - } catch { - fh.readabilityHandler = nil - cont.finish() - return - } - } - if let str = String(data: data, encoding: .utf8), !str.isEmpty { - var lines = str.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - for line in lines { - cont.yield(line) - } - } - } - } - - for await line in stream { - print(line) - } - } } } diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index 41f33d491..8d503329f 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -229,26 +229,42 @@ public actor ContainersService { /// Calculate disk usage for containers /// - Returns: Tuple of (total count, active count, total size, reclaimable size) public func calculateDiskUsage() async -> (Int, Int, UInt64, UInt64) { - await lock.withLock(logMetadata: ["acquirer": "\(#function)"]) { _ in - var totalSize: UInt64 = 0 - var reclaimableSize: UInt64 = 0 - var activeCount = 0 - + // Take a consistent snapshot of the container list under the lock, then + // release it. Walking the bundles takes as long as the containers are + // large, and must not block unrelated create/start/stop/delete calls. + let statuses = await lock.withLock(logMetadata: ["acquirer": "\(#function)"]) { _ in + var statuses = [String: RuntimeStatus]() for (id, state) in await self.containers { - let bundlePath = self.containerRoot.appendingPathComponent(id) - let containerSize = FileManager.default.allocatedSize(of: bundlePath) - totalSize += containerSize - - if state.snapshot.status == .running { - activeCount += 1 - } else { - // Stopped containers are reclaimable - reclaimableSize += containerSize - } + statuses[id] = state.snapshot.status } + return statuses + } + return await self.bundleDiskUsage(of: statuses) + } - return (await self.containers.count, activeCount, totalSize, reclaimableSize) + /// Sum the on-disk size of the given containers' bundles. + /// + /// `nonisolated` so the filesystem walk runs off the actor's executor + /// instead of stalling every other request to this service. + private nonisolated func bundleDiskUsage(of statuses: [String: RuntimeStatus]) async -> (Int, Int, UInt64, UInt64) { + var totalSize: UInt64 = 0 + var reclaimableSize: UInt64 = 0 + var activeCount = 0 + + for (id, status) in statuses { + let bundlePath = self.containerRoot.appendingPathComponent(id) + let containerSize = FileManager.default.allocatedSize(of: bundlePath) + totalSize += containerSize + + if status == .running { + activeCount += 1 + } else { + // Stopped containers are reclaimable + reclaimableSize += containerSize + } } + + return (statuses.count, activeCount, totalSize, reclaimableSize) } /// Get set of image references used by containers (for disk usage calculation) diff --git a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift index 3edd96e4d..e4c7328dc 100644 --- a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift +++ b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift @@ -196,11 +196,14 @@ public actor VolumesService { ) } - return try await lock.withLock { _ in + // Take a consistent snapshot under the locks, then release them. Sizing + // the volumes takes as long as they are large, and must not block + // unrelated volume or container operations. + let (allVolumes, inUseSet) = try await lock.withLock { _ in let allVolumes = try await self.store.list() // Atomically get active volumes with container list - return try await self.containersService.withContainerList(logMetadata: ["acquirer": "\(#function)"]) { containers in + let inUseSet = try await self.containersService.withContainerList(logMetadata: ["acquirer": "\(#function)"]) { containers in var inUseSet = Set() // Find all mounted volumes @@ -212,23 +215,34 @@ public actor VolumesService { } } - var totalSize: UInt64 = 0 - var reclaimableSize: UInt64 = 0 + return inUseSet + } - // Calculate sizes - for volume in allVolumes { - let volumePath = self.volumePath(for: volume.name) - let volumeSize = FileManager.default.allocatedSize(of: URL(fileURLWithPath: volumePath)) - totalSize += volumeSize + return (allVolumes, inUseSet) + } - if !inUseSet.contains(volume.name) { - reclaimableSize += volumeSize - } - } + return await self.diskUsage(of: allVolumes, inUse: inUseSet) + } - return (allVolumes.count, inUseSet.count, totalSize, reclaimableSize) + /// Sum the on-disk size of the given volumes. + /// + /// `nonisolated` so the filesystem walk runs off the actor's executor + /// instead of stalling every other request to this service. + private nonisolated func diskUsage(of volumes: [VolumeConfiguration], inUse: Set) async -> (Int, Int, UInt64, UInt64) { + var totalSize: UInt64 = 0 + var reclaimableSize: UInt64 = 0 + + for volume in volumes { + let volumePath = self.volumePath(for: volume.name) + let volumeSize = FileManager.default.allocatedSize(of: URL(fileURLWithPath: volumePath)) + totalSize += volumeSize + + if !inUse.contains(volume.name) { + reclaimableSize += volumeSize } } + + return (volumes.count, inUse.count, totalSize, reclaimableSize) } private func parseSize(_ sizeString: String) throws -> UInt64 { diff --git a/Tests/ContainerCommandsTests/LogTailerTests.swift b/Tests/ContainerCommandsTests/LogTailerTests.swift new file mode 100644 index 000000000..3e59f8ffa --- /dev/null +++ b/Tests/ContainerCommandsTests/LogTailerTests.swift @@ -0,0 +1,97 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import Testing + +@testable import ContainerCommands + +struct LogTailerTests { + /// Write `contents` to a temporary file and read its last `n` lines back. + private func lastLines(of contents: String, n: Int) throws -> [String] { + let url = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try contents.write(to: url, atomically: true, encoding: .utf8) + defer { try? FileManager.default.removeItem(at: url) } + + let fh = try FileHandle(forReadingFrom: url) + defer { try? fh.close() } + return try LogTailer.lastLines(fh: fh, n: n) + } + + @Test + func returnsTrailingLines() throws { + let lines = try lastLines(of: "one\ntwo\nthree\n", n: 2) + #expect(lines == ["two", "three"]) + } + + @Test + func returnsEveryLineWhenFileIsShorterThanRequested() throws { + let lines = try lastLines(of: "one\ntwo\n", n: 10) + #expect(lines == ["one", "two"]) + } + + @Test + func handlesFinalLineWithoutTrailingNewline() throws { + let lines = try lastLines(of: "one\ntwo", n: 1) + #expect(lines == ["two"]) + } + + @Test + func skipsBlankLines() throws { + let lines = try lastLines(of: "one\n\n\ntwo\n\n", n: 2) + #expect(lines == ["one", "two"]) + } + + @Test + func emptyFileYieldsNoLines() throws { + let lines = try lastLines(of: "", n: 5) + #expect(lines.isEmpty) + } + + @Test + func nonPositiveCountYieldsNoLines() throws { + let lines = try lastLines(of: "one\ntwo\n", n: 0) + #expect(lines.isEmpty) + } + + /// Lines longer than the internal read chunk must not be truncated, and the + /// partial line left at a chunk boundary must not be reported as a line. + @Test + func handlesLinesLongerThanTheReadChunk() throws { + let long = String(repeating: "x", count: 5000) + let contents = "head\n\(long)\ntail\n" + + let lastTwo = try lastLines(of: contents, n: 2) + #expect(lastTwo == [long, "tail"]) + + let lastThree = try lastLines(of: contents, n: 3) + #expect(lastThree == ["head", long, "tail"]) + } + + /// A chunk boundary can split a multi-byte UTF-8 sequence; the surviving + /// lines must still decode correctly. + @Test + func handlesMultiByteCharactersAcrossChunkBoundaries() throws { + let padded = String(repeating: "é", count: 2000) + let contents = "\(padded)\nlast\n" + + let lastOne = try lastLines(of: contents, n: 1) + #expect(lastOne == ["last"]) + + let lastTwo = try lastLines(of: contents, n: 2) + #expect(lastTwo == [padded, "last"]) + } +} From 11c23e6f6c648e869c8e67cc52302e944e8fd9ee Mon Sep 17 00:00:00 2001 From: gnavadev Date: Mon, 27 Jul 2026 10:38:13 -0700 Subject: [PATCH 2/2] Preserve blank lines in log output, buffer partial lines when following. --- Sources/ContainerCommands/LogTailer.swift | 59 ++++++++++++------- .../LogTailerTests.swift | 19 +++++- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Sources/ContainerCommands/LogTailer.swift b/Sources/ContainerCommands/LogTailer.swift index a603dd406..15bb930b3 100644 --- a/Sources/ContainerCommands/LogTailer.swift +++ b/Sources/ContainerCommands/LogTailer.swift @@ -62,7 +62,7 @@ enum LogTailer { try fh.seek(toOffset: offset) let chunk = fh.readData(ofLength: Int(readSize)) - lineCount += Self.countLines(in: chunk, precedingChunk: chunks.last) + lineCount += Self.countNewlines(in: chunk) chunks.append(chunk) } @@ -76,28 +76,25 @@ enum LogTailer { return [] } - let lines = text.components(separatedBy: .newlines).filter { !$0.isEmpty } + var lines = text.components(separatedBy: "\n") + // A trailing newline terminates the final line rather than starting an empty one. + if lines.last?.isEmpty == true { + lines.removeLast() + } return Array(lines.suffix(n)) } - private static func countLines(in chunk: Data, precedingChunk: Data?) -> Int { + private static func countNewlines(in chunk: Data) -> Int { var count = 0 - var previous: UInt8? - for byte in chunk { - if byte != Self.newline, previous == nil || previous == Self.newline { - count += 1 - } - previous = byte - } - - if let last = chunk.last, last != Self.newline, let first = precedingChunk?.first, first != Self.newline { - count -= 1 + for byte in chunk where byte == Self.newline { + count += 1 } return count } private static func followFile(fh: FileHandle) async throws { _ = try fh.seekToEnd() + let pending = LineBuffer() let stream = AsyncStream { cont in fh.readabilityHandler = { handle in let data = handle.availableData @@ -107,15 +104,11 @@ enum LogTailer { } catch { fh.readabilityHandler = nil cont.finish() - return } + return } - if let str = String(data: data, encoding: .utf8), !str.isEmpty { - var lines = str.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - for line in lines { - cont.yield(line) - } + for line in pending.appendAndTakeCompleteLines(data) { + cont.yield(line) } } } @@ -124,4 +117,30 @@ enum LogTailer { print(line) } } + + /// Accumulates bytes across reads so only complete lines are emitted. A read can end + /// mid-line or mid-UTF-8-sequence, so without this a fragment would print as a whole + /// line and a split multi-byte character would discard the entire read. + private final class LineBuffer: @unchecked Sendable { + private let lock = NSLock() + private var buffer = Data() + + func appendAndTakeCompleteLines(_ data: Data) -> [String] { + lock.lock() + defer { lock.unlock() } + + buffer.append(data) + + var lines: [String] = [] + var start = buffer.startIndex + while let index = buffer[start...].firstIndex(of: LogTailer.newline) { + lines.append(String(decoding: buffer[start.. buffer.startIndex { + buffer = Data(buffer[start...]) + } + return lines + } + } } diff --git a/Tests/ContainerCommandsTests/LogTailerTests.swift b/Tests/ContainerCommandsTests/LogTailerTests.swift index 3e59f8ffa..d926b4929 100644 --- a/Tests/ContainerCommandsTests/LogTailerTests.swift +++ b/Tests/ContainerCommandsTests/LogTailerTests.swift @@ -49,10 +49,25 @@ struct LogTailerTests { #expect(lines == ["two"]) } + /// Blank lines are lines. `tail -n` preserves them, and so must this. @Test - func skipsBlankLines() throws { + func preservesBlankLines() throws { + let lines = try lastLines(of: "one\n\ntwo\n", n: 3) + #expect(lines == ["one", "", "two"]) + } + + /// A run of blank lines at the end of the file is still counted and returned. + @Test + func preservesTrailingBlankLines() throws { let lines = try lastLines(of: "one\n\n\ntwo\n\n", n: 2) - #expect(lines == ["one", "two"]) + #expect(lines == ["two", ""]) + } + + /// A file of nothing but newlines yields that many blank lines. + @Test + func handlesFileOfOnlyBlankLines() throws { + let lines = try lastLines(of: "\n\n\n\n\n", n: 3) + #expect(lines == ["", "", ""]) } @Test