Gate the direct SwiftInfo pass-throughs on the layering features - #1879
Merged
Conversation
b054a97 (bazelbuild#1780) re-exports dependency `SwiftInfo`s as *direct* in two places so the Swift layering check can see through targets that do not produce a module map: 1. `swift_clang_module_aspect`'s module-map-less path, as `SwiftInfo(direct_swift_infos = direct_swift_infos + swift_infos)` 2. `swift_library_group`, as `SwiftInfo(direct_swift_infos = get_providers(deps, SwiftInfo), ...)` Both are semantically fine but quadratic at scale, because `_swift_info_init` materializes each direct provider's `direct_modules` into a fresh per-node list, and lists do not share structure the way depsets do: - Through chains of module-map-less aggregates (common with macro-generated wrapper targets), each level's `direct_modules` holds a fresh copy of everything below it. - Through `swift_library_group`, every consumer of a group aggregating N modules loops over all N in `compilation_context_for_explicit_module_compilation` and merges N compilation contexts via `cc_common.merge_compilation_contexts`, per compile, with no memoization. Measured on a large monorepo (~370k configured targets, explicit modules enabled): full-app analysis went from 23 minutes followed by an OOM at a 64 GB heap inside `depset()` in `_swift_info_init` down to 54 seconds, and a single leaf `swift_library` consumer went from an OOM at 30 GB to ~18 s / 325 MB, against ~2.4 GB retained on rules_swift 3.x. A Starlark CPU profile of the ungated run attributed >60% of analysis CPU to `compilation_context_for_explicit_module_compilation` and ~30% to the aspect's `_handle_module`. Only the layering checks consume the widened direct set, so gate both pass-throughs on `swift.layering_check_swift` / `swift.layering_check_for_c_deps` and restore the pre-bazelbuild#1780 direct/transitive split otherwise. Dependencies still reach consumers through `transitive_modules`, so compilation is unaffected. The existing layering-check tests enable those features and therefore still exercise the pass-through path. Note this gates the cost rather than removing it: a consumer that enables a layering check on a large module graph still pays the quadratic materialization. Making the layering check consume a depset instead of materialized direct lists would be the complete fix.
mauriciogg
requested review from
aaronsky,
adincebic,
brentleyjones,
keith and
luispadron
as code owners
August 3, 2026 19:13
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
3 participants
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.
b054a97 (#1780) re-exports dependency
SwiftInfos as direct in two places so the Swift layering check can see through targets that do not produce a module map:swift_clang_module_aspect's module-map-less path, asSwiftInfo(direct_swift_infos = direct_swift_infos + swift_infos)swift_library_group, asSwiftInfo(direct_swift_infos = get_providers(deps, SwiftInfo), ...)This is causing OOM errors in Blaze in our repo ~350K targets since the list is materialized for every single target. This change gates the flattening of the
swift_infoson the layering check feature (which we have disabled) so we can rebase our fork to the latest version.