Repository navigation
[Caching] Read a prefix-mapped binary module from its resolved path - #2258
Open
Dharin-shah wants to merge 1 commit into
Open
Dharin-shah wants to merge 1 commit into
Dharin-shah wants to merge 1 commit into
Conversation
With compilation caching and a prefix map, each Swift module dependency is recorded by its abstract name, `Lib.swiftmodule`, with no directory. Since swiftlang#2128 the currency check resolves that name through externalModulePathMap, so the driver sees that the module is newer, but integrateIncrementalImport still reads the bare name. The read fails and the driver falls back to invalidating every input that uses the module, so an interface change to one declaration recompiles all of them. Read the dependency from the path the name resolves to, the same lookup the currency check uses, and keep the abstract name as the dependency's identity in the graph. Without a map entry the source is unchanged. The new test builds a fingerprinted binary module from source, imports it from two files and changes a declaration only one of them uses. Before this change the cachingPrefixMapped case recompiled both files. Fixes swiftlang#2236
Dharin-shah
requested review from
DougGregor,
artemcm,
compnerd and
owenv
as code owners
November 7, 2026 21:11
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.
Following up on my comment in #2236. With caching and a prefix map, a binary module dependency is recorded by its abstract name (
Lib.swiftmodule). Since #2128 the currency check resolves that name throughexternalModulePathMap, butintegrateIncrementalImportstill reads the bare name, so the read fails and the driver invalidates every input that uses the module 😅The fix reads from the resolved path, the same lookup the currency check uses, and keeps the abstract name as the dependency's identity in the graph. Without a map entry nothing changes.
I think the existing caching tests miss this because their dependencies come from interfaces, which have no fingerprints, so they take the fallback either way. The new test builds a fingerprinted binary module from source, imports it from two files and changes a declaration only one of them uses. On main the
cachingPrefixMappedcase recompiles both files; with this, onlymain.swift, like theexplicitandcachingcases.For scale, on a 250-module app with Xcode 27.2, adding a public function to a module with 13 importers invalidated 699 inputs and took 40 s with caching and prefix mapping, against 61 inputs and 15 s without (one build each, before this change).
Built and tested locally with Xcode 27.2: the new test, the incremental and caching suites and the whole package pass.
Fixes #2236.