Skip to content

test: drop display-name-only @Suite attributes that make the test module compile quadratically - #3129

Merged
datlechin merged 6 commits into
mainfrom
perf/test-suite-compile-time
Sep 25, 2026
Merged

datlechin merged 6 commits into
mainfrom
perf/test-suite-compile-time

Conversation

@datlechin

@datlechin datlechin commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

The macOS Tests "Build for testing" job spends 46 to 54 of its 60 minutes building, and the TableProTests target is most of that. This PR removes the part of the target's compile time that grows with the square of the number of test suites, and adds a check so it does not come back.

Problem

Measured on the CI runner (macos-26, Xcode 26.4.1, Swift 6.3, 3 vCPU, 7 GB) in diagnostic runs 36089300367, 36093890303 and 36096016980:

  • The only swift-frontend job over 120 s in the whole build is the TableProTests emit-module job, at 544 to 626 s. Every app batch finishes in 37 s or less.
  • That job grew with the number of top-level @Suite attributes: 238 s at 1,644, 340 s at 1,826, 622 s at 2,163, 608 to 626 s at 2,336 (column-zero counts; 27 more sat behind @MainActor on the same line).
  • 51 to 55% of its main-thread samples sit in one stack: SourceLookupCache::lookupValue → MissingDecl::forEachMacroExpandedDecl → Decl::visitAuxiliaryDecls → SourceManager::findBufferContainingLocInternal.

Cause

@Suite is a peer macro, and a peer macro may introduce uniquely named declarations. So SourceLookupCache::populateAuxiliaryDeclCache files every top-level @Suite under one placeholder for macro-generated unique names (MacroDecl::getIntroducedNames). Each @Suite expansion emits a generator, an accessor and a test content record that refer to each other by those unique names, and every lookup of one walks the whole placeholder list, visiting the expansion of every other top-level @Suite in the module. That is quadratic in the emit-module job, and it costs every 25-file compile batch time in proportion to the suite count.

A nested @Suite and a @Test expand into members of a type, so they go through member lookup and avoid the quadratic term. A nested @Suite is not free, though: in the reproducer it still adds emit-module time that grows linearly, about 60% more at 2,000 suites, so the fix leaves types unannotated rather than nesting them.

Change

  • Removes the 2,200 top-level @Suite attributes in TableProTests whose only argument was a display name: 2,173 lines that were exactly @Suite("…"), and 27 @MainActor @Suite("…") lines, which keep @MainActor. 1,666 files, attribute lines only.
  • Keeps the 137 top-level @Suite attributes that carry a trait (.serialized, .enabled(if:)) and the 53 nested ones.
  • Swift Testing finds a type's @Test functions without @Suite, so every test still runs under the same identifier.

Measured

  • CI, same runner, same run (36096016980): the emit-module job went from 544 s to 290 s, the target's compile batches, summed, from 3,958 s to 2,373 s (-40%), and the target's build phase from 25.7 min to 18.6 min.

  • Locally (Xcode 27.1, Swift 6.4, M4 Pro), replaying the target's emit-module job alone on this base, twice each way: 104.8 s and 104.1 s before, 73.8 s and 72.7 s after (-30%).

  • This PR's run 36125444612 against main's run 36121207600 on c4833e6, and against fix(ci): end the compile stall, the package test deadlock and the unreachable cell viewer that keep main red #3126's last run 36097535460 (whose runner compiled the plugins as fast as this one did):

    main fix(ci): end the compile stall, the package test deadlock and the unreachable cell viewer that keep main red #3126 this PR
    Build for testing job 51:07 45:48 36:05
    "Build the app and both test bundles" step 41:27 38:19 29:12
    "Compile every plugin" step (runner speed) 6:18 4:11 4:09
    Longest silent stretch in the build step log 489 s 578 s 262 s

    The silent stretch is the window the diagnostic runs tied to the TableProTests emit-module job, the one that grew with the suite count. Runners vary: main's run compiled the plugins 50% slower than the other two.

Trade-off

  • Xcode's test navigator and the result bundle show the type name (MultiRowEditStateTests) instead of the display name ("MultiRowEditState") for those 2,200 suites. Test identifiers, -only-testing filters and the quarantine list already use type names and are unchanged.
  • It roughly halves the emit-module job rather than removing it. What remains was not broken down here. The job's footprint was 4.4 to 7.1 GB on a 7 GB runner, so memory pressure is a factor too.

Guard

Display-name-only @Suite was the house style, so it would come back. scripts/ci/check-test-suite-attributes.py fails Repo Hygiene on any top-level @Suite in TableProTests without a trait (a display name alone, a bare @Suite, or @Suite()), and prints file:line, the reason and the fix. It tracks brace depth past comments and string literals, so a suite inside an #if block is still top-level and a nested one is not. A file whose braces do not balance as it reads them (an #if whose branches open a scope differently, a regex literal holding a brace) fails the check instead of passing unread; no file in TableProTests does that today. It scans TableProTests only: the cost is quadratic in each module's own count, and every other test module holds 108 top-level suites or fewer. On main it reports 2,200 sites, the same set grep finds; on this branch, none.

test_check_test_suite_attributes.py pins it with fixtures, and each of five scanner mutations (no brace depth, no string skipping, column zero only, non-nesting block comments, @Suite( only) fails at least one case. Repo Hygiene now also runs when TableProTests/**/*.swift changes. CLAUDE.md records the rule under Performance Pitfalls, and the fix-issue skill repeats it where tests get written.

Verification

  • Test identity: xcodebuild test-without-building -enumerate-tests before and after the change on this base lists 23,559 identifiers in 2,517 suites, and the two JSON files are byte-identical.
  • verify.sh test on 26 suites across AWS, Core, Database, Models, Plugins, ViewModels and Views: 699 of 699 cases passed, and every suite ran all its enumerated cases. 20 of them lost their @Suite (5 of those were @MainActor @Suite, 6 hold nested suites), 6 keep one with a trait (.serialized, and GitIntegrationTests with .enabled(if:)).
  • swiftlint lint --strict on the 1,666 changed files: the same 668 violations before and after, keyed by file and message, none new. TableProTests is outside SwiftLint's included: and these predate this change.
  • python3 scripts/ci/test_check_test_suite_attributes.py: 7 passed. actionlint on the workflow: clean.
  • CI on 8762d8d: macOS Tests run 36125444612 passed every job, macOS Tests Gate included, in 65 min end to end against 82 min for main's run 36121207600. Unit tests printed 22,973 passing cases against main's 22,972; the one extra line is PluginGrantSQLBuilderTests/dropsHostilePrivilegeNames(), which main's xcbeautify output drops while its suite still passes. Repo Hygiene ran both new steps and passed, and Docs and iOS Tests passed.
  • No CHANGELOG entry: nothing a user sees changes.

Upstream: filed as swiftlang/swift#92638, with a standalone reproducer.

@datlechin
datlechin marked this pull request as ready for review September 25, 2026 13:53
@datlechin
datlechin merged commit 1d6b99e into main Sep 25, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant