test: drop display-name-only @Suite attributes that make the test module compile quadratically - #3129
Merged
Merged
Conversation
…ule compile quadratically
datlechin
marked this pull request as ready for review
September 25, 2026 13:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The macOS Tests "Build for testing" job spends 46 to 54 of its 60 minutes building, and the
TableProTeststarget 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:
swift-frontendjob over 120 s in the whole build is theTableProTestsemit-module job, at 544 to 626 s. Every app batch finishes in 37 s or less.@Suiteattributes: 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@MainActoron the same line).SourceLookupCache::lookupValue→MissingDecl::forEachMacroExpandedDecl→Decl::visitAuxiliaryDecls→SourceManager::findBufferContainingLocInternal.Cause
@Suiteis a peer macro, and a peer macro may introduce uniquely named declarations. SoSourceLookupCache::populateAuxiliaryDeclCachefiles every top-level@Suiteunder one placeholder for macro-generated unique names (MacroDecl::getIntroducedNames). Each@Suiteexpansion 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@Suitein 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
@Suiteand a@Testexpand into members of a type, so they go through member lookup and avoid the quadratic term. A nested@Suiteis 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
@Suiteattributes inTableProTestswhose 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.@Suiteattributes that carry a trait (.serialized,.enabled(if:)) and the 53 nested ones.@Testfunctions 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):
The silent stretch is the window the diagnostic runs tied to the
TableProTestsemit-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
MultiRowEditStateTests) instead of the display name ("MultiRowEditState") for those 2,200 suites. Test identifiers,-only-testingfilters and the quarantine list already use type names and are unchanged.Guard
Display-name-only
@Suitewas the house style, so it would come back.scripts/ci/check-test-suite-attributes.pyfails Repo Hygiene on any top-level@SuiteinTableProTestswithout a trait (a display name alone, a bare@Suite, or@Suite()), and printsfile:line, the reason and the fix. It tracks brace depth past comments and string literals, so a suite inside an#ifblock is still top-level and a nested one is not. A file whose braces do not balance as it reads them (an#ifwhose branches open a scope differently, a regex literal holding a brace) fails the check instead of passing unread; no file inTableProTestsdoes that today. It scansTableProTestsonly: 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 setgrepfinds; on this branch, none.test_check_test_suite_attributes.pypins 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 whenTableProTests/**/*.swiftchanges. CLAUDE.md records the rule under Performance Pitfalls, and the fix-issue skill repeats it where tests get written.Verification
xcodebuild test-without-building -enumerate-testsbefore 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 teston 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, andGitIntegrationTestswith.enabled(if:)).swiftlint lint --stricton the 1,666 changed files: the same 668 violations before and after, keyed by file and message, none new.TableProTestsis outside SwiftLint'sincluded:and these predate this change.python3 scripts/ci/test_check_test_suite_attributes.py: 7 passed.actionlinton the workflow: clean.macOS Tests Gateincluded, 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 isPluginGrantSQLBuilderTests/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.Upstream: filed as swiftlang/swift#92638, with a standalone reproducer.