Skip to content

[Caching] Handle emit module job correctly for swift caching #1602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 10, 2024

Conversation

cachemeifyoucan
Copy link
Contributor

There are two situations when emiting modules are not correctly handled when swift caching is enabled:

  • When only -emit-module task is requested from driver, in this case, we are contructing a CompileJob but only generating module output. SwiftDriver still expects all swift inputs will have a cache key even only the first file is generating outputs and has a cache key.
  • When using -experimental-emit-module-separately, then swift-driver needs to construct a EmitModuleJob, which wasn't taught the concept of caching and not producing any cache key.

rdar://127768967

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please test

@@ -119,14 +119,21 @@ extension Driver {
commandLine.appendPath(abiPath.file)
outputs.append(abiPath)
}
let cacheContributingInputs = inputs.enumerated().reduce(into: [(TypedVirtualPath, Int)]()) { result, input in
// only the first swift input contributes to an emit module job.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by contribute here? I assume it's specific to the cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way we create cache key for compilation is we have one cache key per input file, and each cache key contains all the output files for that specific input (like .o, .d, .diag). All the module level outputs are attached to the first swift input file.

This is just computing all the input file that has a cache key because there is an output produced that linked to the cache key for this input file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add this to the comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the same function in CompileJob which is more complicated and worth explanation.

@@ -119,14 +119,21 @@ extension Driver {
commandLine.appendPath(abiPath.file)
outputs.append(abiPath)
}
let cacheContributingInputs = inputs.enumerated().reduce(into: [(TypedVirtualPath, Int)]()) { result, input in
// only the first swift input contributes to an emit module job.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add this to the comment here?

There are two situations when emiting modules are not correctly handled
when swift caching is enabled:
* When only `-emit-module` task is requested from driver, in this case,
  we are contructing a `CompileJob` but only generating module output.
  SwiftDriver still expects all swift inputs will have a cache key even
  only the first file is generating outputs and has a cache key.
* When using `-experimental-emit-module-separately`, then swift-driver
  needs to construct a `EmitModuleJob`, which wasn't taught the concept of
  caching and not producing any cache key.

rdar://127768967
@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please test

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please test windows platform

@cachemeifyoucan cachemeifyoucan merged commit 5833856 into swiftlang:main May 10, 2024
3 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.

3 participants