From 41ceca483ecc8f5f996a6e7d0a69a6f740f0067a Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Sun, 13 Sep 2026 03:06:22 +0300 Subject: [PATCH] test(menubar): make the localization scan cost milliseconds, not seconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit swift-testing runs suites concurrently in one process, so a slow test steals CPU from every wall-clock assertion running beside it. ServeConnectionTests asserts that cancelling a hung request returns inside 500 ms (#1333), and on PR #1289 it overshot by 16 ms twice in a row while LocalizationCoverageTests ran next to it. That scan was burning about 21 s of CPU in the unoptimised build `swift test` uses. None of that cost was essential — the tree is 88 files and 1.3 MB — it was four algorithmic mistakes: - the file was walked once per call-site pattern, 26 times over; - every comparison built a fresh Array slice at each character position, tens of millions of allocations per run; - enclosingTypeName rescanned the file from the top for every display-label property it found, which is quadratic; - and the suite's four whole-tree tests each re-read and re-tokenized every file. The scanner now blanks comments in place over a UTF-8 buffer, walks each file exactly once behind a one- and two-byte dispatch table (`.` alone is about one byte in thirty and carries ten candidate patterns), tracks the enclosing type as the walk passes it, builds the line index only when there is a finding to report, and memoises the scan so the four tests pay for one. Measured on this host, debug build as CI runs it (load average ~35 from concurrent builds, so wall time is contention; thread CPU is the stable figure): before after thread CPU, whole suite 20.6 - 22.4 s 0.20 - 0.25 s wall, same runs 133 - 200 s 0.22 - 0.86 s release build, CPU 6.2 s ~25 ms Detection is unchanged, and this is only a performance fix: - all 22 existing LocalizationCoverageTests pass unmodified against the new scanner (run through a Testing shim, since this host has no Testing module); - a differential run of main's scanner against this one over every source file, three mutations of each that force the positive paths (every L( un-routed, stripped, and so on), and 25 edge cases including nested block comments, multiline and unterminated literals and non-ASCII text, agrees on all 313 findings and 1,406 keys. A new test pins the cost: it prints the scan's wall and thread CPU once, and fails above 2 s of thread CPU — ten times the current figure, so it trips on an algorithmic regression rather than a slow runner. It asserts thread CPU, not wall time, because a wall-clock ceiling on a shared runner would be the very flake this exists to prevent, and not process CPU, which would bill it for every test running concurrently. Mutation-checked: against main's original scanner it fails at 20.6 s. Refs #1333 --- .../LocalizationCoverageTests.swift | 36 + .../LocalizationSourceScanner.swift | 895 ++++++++++++------ 2 files changed, 619 insertions(+), 312 deletions(-) diff --git a/mac/Tests/CodeBurnMenubarTests/LocalizationCoverageTests.swift b/mac/Tests/CodeBurnMenubarTests/LocalizationCoverageTests.swift index 680b499e3..7366c4d43 100644 --- a/mac/Tests/CodeBurnMenubarTests/LocalizationCoverageTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/LocalizationCoverageTests.swift @@ -104,6 +104,42 @@ struct LocalizationCoverageTests { ) } + /// A generous ceiling on what the whole-tree scan costs. + /// + /// swift-testing runs suites concurrently in one process, so this scan's + /// cost is paid by every wall-clock assertion running beside it — + /// `ServeConnectionTests` checks that cancelling a hung request returns + /// inside 500 ms (#1333). In the unoptimised build `swift test` uses, the + /// scan once burned about 21 s of CPU across the four tests above; it now + /// takes about 0.2 s, once, because the result is memoised. The ceiling is + /// ten times that, so it trips on an algorithmic regression — a pattern + /// loop rescanning the file, an allocation per byte — and never on a merely + /// slow runner. + /// + /// Measured in the scanning thread's own CPU time. Wall time on a shared + /// runner is noise, and would make this test the flake it exists to + /// prevent; process CPU would bill it for every test running concurrently. + @Test("the whole-tree scan stays cheap enough to run beside timing tests") + func scanStaysCheap() throws { + let scan = try LocalizationSourceScanner.scan(directory: Self.sourcesDirectory) + print( + "LocalizationSourceScanner: \(scan.fileCount) files, \(scan.byteCount / 1024) KB, " + + "wall \(Int((scan.wallSeconds * 1000).rounded())) ms, " + + "thread CPU \(Int((scan.cpuSeconds * 1000).rounded())) ms" + ) + #expect(scan.fileCount > 50, "the scan read \(scan.fileCount) files; a cheap scan of nothing proves nothing") + #expect( + scan.cpuSeconds < 2.0, + """ + the localization scan took \(scan.cpuSeconds) s of CPU (ceiling 2 s). It runs \ + concurrently with wall-clock assertions such as ServeConnectionTests' 500 ms \ + cancellation budget (#1333), so a slow scan fails other suites. Look for a \ + per-pattern rescan of the file, a per-byte allocation, or a backward walk \ + per finding in LocalizationSourceScanner. + """ + ) + } + // MARK: - The scanner's own rules // // The guard above is only as good as these: a scanner that quietly stops diff --git a/mac/Tests/CodeBurnMenubarTests/LocalizationSourceScanner.swift b/mac/Tests/CodeBurnMenubarTests/LocalizationSourceScanner.swift index c13e1cf07..dc6381da6 100644 --- a/mac/Tests/CodeBurnMenubarTests/LocalizationSourceScanner.swift +++ b/mac/Tests/CodeBurnMenubarTests/LocalizationSourceScanner.swift @@ -23,6 +23,30 @@ import Foundation /// than inside the test so the standalone `swiftc` harness (this host cannot run /// `swift test`) can exercise the same code the suite does, instead of a /// reimplementation that could drift from it. +/// +/// # Why it walks bytes, once +/// +/// swift-testing runs suites concurrently in one process, so a slow test is not +/// merely slow: it steals CPU from every wall-clock assertion running beside it. +/// `ServeConnectionTests` asserts that cancelling a hung request returns inside +/// 500 ms (#1333), and this scan used to burn 16 s of CPU next to it. +/// +/// The cost was all algorithmic, not essential — the tree is 88 files and 1.3 MB: +/// +/// - the file was walked once per call-site pattern, 26 times over; +/// - every comparison built a fresh `Array` slice at each character position, +/// some 34 million allocations per run; +/// - `enclosingTypeName` rescanned the whole file from the top for every +/// display-label property it found, which is quadratic; +/// - and the four whole-tree scans the suite performs each re-read every file. +/// +/// It now blanks comments in place, walks each file exactly once with a +/// first-byte dispatch table, tracks the enclosing type as it passes it, and +/// memoises the result so the suite's four scans cost one. Every pattern is +/// ASCII; literal text is decoded only for the literals actually found. +/// +/// Detection is unchanged. This is a performance fix, and +/// `LocalizationCoverageTests` is its oracle. enum LocalizationSourceScanner { // MARK: - What counts as user-facing @@ -86,86 +110,6 @@ enum LocalizationSourceScanner { /// `%@`, which is the routed form this scanner is asking for. static let untranslatableWords: Set = ["codeburn", "tok", "usd"] - // MARK: - Findings - - struct Finding: Equatable, CustomStringConvertible { - let file: String - let line: Int - let callSite: String - let literal: String - - var description: String { - "\(file):\(line): \(callSite)\"\(literal)\" is shown to the user but never reaches the catalog — wrap it in L(\"…\")" - } - } - - // MARK: - Scanning - - /// Every user-facing literal in `directory` that is not routed through `L(…)`. - static func unroutedLiterals(in directory: URL) throws -> [Finding] { - var findings: [Finding] = [] - for file in try swiftFiles(in: directory) { - let source = try String(contentsOf: file, encoding: .utf8) - findings += unroutedLiterals( - inSource: source, - fileName: file.lastPathComponent - ) - } - return findings.sorted { - ($0.file, $0.line, $0.literal) < ($1.file, $1.line, $1.literal) - } - } - - static func swiftFiles(in directory: URL) throws -> [URL] { - guard let walker = FileManager.default.enumerator( - at: directory, - includingPropertiesForKeys: nil - ) else { return [] } - return walker - .compactMap { $0 as? URL } - .filter { $0.pathExtension == "swift" } - .sorted { $0.path < $1.path } - } - - /// The scan for one file's text. Split out so the rules are testable - /// against a source snippet rather than the repository. - static func unroutedLiterals(inSource source: String, fileName: String) -> [Finding] { - let code = Array(strippingComments(source)) - var findings: [Finding] = [] - - for callSite in userFacingCallSites { - let needle = Array(callSite) - var index = 0 - while index + needle.count <= code.count { - guard Array(code[index..<(index + needle.count)]) == needle, - startsAWord(needle, at: index, in: code) else { - index += 1 - continue - } - var cursor = index + needle.count - // The argument may be on the next line; whitespace is not a - // reason to stop looking for it. - while cursor < code.count, code[cursor].isWhitespace { cursor += 1 } - if cursor < code.count, code[cursor] == "\"", - let literal = stringLiteral(in: code, startingAt: cursor), - needsTranslation(literal.value) { - findings.append( - Finding( - file: fileName, - line: lineNumber(of: index, in: code), - callSite: callSite, - literal: literal.value - ) - ) - } - index += needle.count - } - } - // Call sites are scanned one kind at a time, so sort back into reading - // order — a failure message that jumps around the file is hard to act on. - return findings.sorted { ($0.line, $0.literal) < ($1.line, $1.literal) } - } - // MARK: - Display-label properties /// Computed `String` properties this codebase uses to give an enum its @@ -209,68 +153,422 @@ enum LocalizationSourceScanner { "Tier.displayName", ] - /// Bare literals returned from a display-label property. - static func unroutedLabelProperties(in directory: URL) throws -> [Finding] { - var findings: [Finding] = [] - for file in try swiftFiles(in: directory) { - let source = try String(contentsOf: file, encoding: .utf8) - findings += unroutedLabelProperties( - inSource: source, - fileName: file.lastPathComponent - ) + // MARK: - Findings + + struct Finding: Equatable, CustomStringConvertible { + let file: String + let line: Int + let callSite: String + let literal: String + + var description: String { + "\(file):\(line): \(callSite)\"\(literal)\" is shown to the user but never reaches the catalog — wrap it in L(\"…\")" + } + } + + /// Everything one walk of the tree produces, plus what it cost. + struct Scan: Sendable { + var unroutedLiterals: [Finding] = [] + var unroutedLabelProperties: [Finding] = [] + var requestedKeys: Set = [] + var fileCount = 0 + var byteCount = 0 + /// Wall time and CPU time for this scan. `LocalizationCoverageTests` + /// prints them once and fails if the CPU figure regresses past a + /// ceiling, because the cost of this scan is a property of the whole + /// suite, not just of this test. + var wallSeconds: Double = 0 + /// CPU consumed by the scanning thread alone. Process CPU would bill + /// this scan for every test swift-testing runs beside it; the walk is + /// synchronous, so it never leaves the thread it is measured on. + var cpuSeconds: Double = 0 + } + + // MARK: - Byte classification + // + // Every pattern this scanner matches is ASCII. A byte at or above 0x80 is a + // UTF-8 lead or continuation byte, and counts as a letter so a match can + // never start in the middle of non-ASCII text. + + @inline(__always) + static func isLetter(_ b: UInt8) -> Bool { + (b >= 0x41 && b <= 0x5A) || (b >= 0x61 && b <= 0x7A) || b >= 0x80 + } + + @inline(__always) + static func isDigit(_ b: UInt8) -> Bool { b >= 0x30 && b <= 0x39 } + + @inline(__always) + static func isIdentifier(_ b: UInt8) -> Bool { + isLetter(b) || isDigit(b) || b == UInt8(ascii: "_") + } + + @inline(__always) + static func isSpace(_ b: UInt8) -> Bool { + b == 0x20 || b == 0x09 || b == 0x0A || b == 0x0D + } + + @inline(__always) + static func isUppercase(_ b: UInt8) -> Bool { b >= 0x41 && b <= 0x5A } + + /// Non-allocating prefix comparison. The previous shape, + /// `Array(code[i.. Bool { + guard index + needle.count <= code.count else { return false } + for k in 0.. Int { + let starts: [Int] + if let cached = lineStarts { + starts = cached + } else { + var built = [0] + built.reserveCapacity(code.count / 30) + for i in 0.. Scan { + try cache.scan(directory) + } + + /// Every user-facing literal in `directory` that is not routed through `L(…)`. + static func unroutedLiterals(in directory: URL) throws -> [Finding] { + try scan(directory: directory).unroutedLiterals + } + + /// Bare literals returned from a display-label property. + static func unroutedLabelProperties(in directory: URL) throws -> [Finding] { + try scan(directory: directory).unroutedLabelProperties + } + + /// Every key passed to `L(…)` anywhere under `directory`. + /// + /// The other direction of the same guard: `unroutedLiterals` catches copy + /// that never became a key, this catches a key that never became an entry. + /// Both ship English in a zh-Hans build, and neither is visible to the + /// compiler or to a catalog-versus-catalog diff. + static func requestedKeys(in directory: URL) throws -> Set { + try scan(directory: directory).requestedKeys + } + + /// Swift sources under `directory`, skipping hidden trees such as `.build` + /// and anything inside a nested package. + static func swiftFiles(in directory: URL) throws -> [URL] { + guard let walker = FileManager.default.enumerator( + at: directory, + includingPropertiesForKeys: nil, + options: [.skipsHiddenFiles, .skipsPackageDescendants] + ) else { return [] } + return walker + .compactMap { $0 as? URL } + .filter { $0.pathExtension == "swift" } + .sorted { $0.path < $1.path } + } + + // The per-source entry points. The suite uses these to test the rules + // against a snippet rather than the repository. + + /// The scan for one file's text. Split out so the rules are testable + /// against a source snippet rather than the repository. + static func unroutedLiterals(inSource source: String, fileName: String) -> [Finding] { + analyze(ScannedFile(name: fileName, source: [UInt8](source.utf8))).unroutedLiterals + } + static func unroutedLabelProperties(inSource source: String, fileName: String) -> [Finding] { - let code = Array(strippingComments(source)) - var findings: [Finding] = [] - - for property in displayLabelProperties { - let needle = Array("var \(property): String") - var index = 0 - while index + needle.count <= code.count { - guard Array(code[index..<(index + needle.count)]) == needle else { - index += 1 - continue - } - let owner = enclosingTypeName(before: index, in: code) - let qualified = "\(owner).\(property)" - index += needle.count - guard !untranslatedLabelProperties.contains(qualified) else { continue } - guard let body = propertyBody(in: code, after: index) else { continue } - for literal in valuePositionLiterals(in: Array(code[body])) where needsTranslation(literal.value) { - findings.append( + analyze(ScannedFile(name: fileName, source: [UInt8](source.utf8))).unroutedLabelProperties + } + + static func requestedKeys(inSource source: String) -> Set { + analyze(ScannedFile(name: "", source: [UInt8](source.utf8))).requestedKeys + } + + // MARK: - Dispatch tables + + /// Call-site patterns bucketed by first byte, so one walk tests only the two + /// or three patterns that could start here instead of walking the file once + /// per pattern. + private static let callSiteTable: [[(needle: [UInt8], text: String)]] = { + var table = [[(needle: [UInt8], text: String)]](repeating: [], count: 256) + for site in userFacingCallSites { + let bytes = [UInt8](site.utf8) + guard let first = bytes.first else { continue } + table[Int(first)].append((bytes, site)) + } + // Longest first, so `.accessibilityLabel(` wins over any prefix of it. + for i in table.indices { table[i].sort { $0.needle.count > $1.needle.count } } + return table + }() + + /// `var displayName: String` and friends, matched whole. + private static let labelPropertyNeedles: [(needle: [UInt8], property: String)] = + displayLabelProperties.map { ([UInt8]("var \($0): String".utf8), $0) } + + private static let typeKeywords: [[UInt8]] = + ["enum ", "struct ", "final class ", "class ", "extension "].map { [UInt8]($0.utf8) } + + /// Which of the four jobs a byte could possibly begin. Consulted once per + /// byte, so the overwhelming majority cost one array read and one test. + private static let interesting: [UInt8] = { + var flags = [UInt8](repeating: 0, count: 256) + for site in userFacingCallSites { + if let first = site.utf8.first { flags[Int(first)] |= 0b0001 } + } + flags[Int(UInt8(ascii: "v"))] |= 0b0010 // var