Add feature for partially explicit modules #763
Conversation
|
Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/236 (must be Lyft employee to view) |
swift/internal/feature_names.bzl
Outdated
| @@ -202,6 +202,11 @@ SWIFT_FEATURE_USE_RESPONSE_FILES = "swift.use_response_files" | |||
| # when access to those paths involves traversing a networked file system. | |||
| SWIFT_FEATURE_VFSOVERLAY = "swift.vfsoverlay" | |||
|
|
|||
| # If enabled, Swift compilation actions will use a explicit module map file for | |||
| # discovering dependencies. This is faster than using search paths when you | |||
| # have many dependencies. This doesn't apply to system dependencies. | |||
There was a problem hiding this comment.
This doesn't need to go in the comment, but what is the tipping point for "many"?
There was a problem hiding this comment.
Hard to say, for us at this point with >2k modules (not all in every dependency tree) this improves build performance by >50%, so if you have more than maybe 20 it's worth a try?
There was a problem hiding this comment.
Is there any downside to just using this always when on Swift 5.6+?
There was a problem hiding this comment.
not that I know of yet, I don't recall why we never defaulted the VFS overlay to on 🤔 , but I also haven't tested incremental with this
|
Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/237 (must be Lyft employee to view) |
|
Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/238 (must be Lyft employee to view) |
keith
force-pushed
the
ks/add-feature-for-partially-explicit-modules
branch
from
66746bb to
a930545
Compare
|
Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/348 (must be Lyft employee to view) |
keith
force-pushed
the
ks/add-feature-for-partially-explicit-modules
branch
from
a930545 to
27a1259
Compare
|
ok I pushed a new version here where I manually pulled changes from the upstream commit, but also left the VFS feature around for existing users. we can probably remove it as upstream did once we're confident this works without downsides and we don't care about older swift versions |
…e explicit Swift module JSON file supported by Swift 5.6 (but not enabled on any toolchain yet). This file is actually supported in earlier versions of Swift, but 5.6 is the first version where it doesn't require *all* modules (i.e., system modules) to be specified explicitly as well. PiperOrigin-RevId: 456822150
keith
force-pushed
the
ks/add-feature-for-partially-explicit-modules
branch
from
27a1259 to
73231cf
Compare
keith
requested review from
BalestraPatrick,
segiddins and
thii
as code owners
As of Swift 5.6 and this PR swiftlang/swift#39887 the new json format used for entirely explicit module builds can be used without entirely disabling implicit modules. This provides an alternative to VFS overlays for discovering dependencies without `-I` search paths. This is theoretically about the same but I expect this file format to have better support in general than VFS overlays for things like the new incremental compilation support, which currently doesn't support VFS overlays, and this shouldn't have any downsides vs VFS files. This format does support some more keys than just the 2 we pass, as far as I can tell they are unused today, so I'm opting not to pass them. We may have to revisit this in the future. This kind of cherry picks 1666670 but I kept the VFS feature for now for legacy users.
As of Swift 5.6 and this PR swiftlang/swift#39887
the new json format used for entirely explicit module builds can be used
without entirely disabling implicit modules. This provides an
alternative to VFS overlays for discovering dependencies without
-Isearch paths. This is theoretically about the same but I expect this
file format to have better support in general than VFS overlays for
things like the new incremental compilation support, which currently
doesn't support VFS overlays, and this shouldn't have any downsides vs
VFS files.
This format does support some more keys than just the 2 we pass, as far
as I can tell they are unused today, so I'm opting not to pass them. We
may have to revisit this in the future.
This kind of cherry picks 1666670 but I kept the VFS feature for now for legacy users.