Skip to content

Re-land [Planning] Avoid batching compile job twice #1829

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
Mar 7, 2025

Conversation

cachemeifyoucan
Copy link
Contributor

This re-land the change in 7ba9d48 with compatibility fixes and additional tests.

The fix makes sure the skipped jobs are returned in the planning as well for compatibility reasons. Not after this change:

For the build systems that use swift-driver that doesn't support incremental build (swiftpm), asking for an incremental planning will still get the full list of jobs, but the skipped jobs might not be batched. The planned jobs are still complete, but might not be optimally batched.

For the build systems that supports incremental build (swift-build), the build planning is extracted from incremental state directly. But note swift-driver lazily wrapped the compile jobs in Executor so swift-driver can't return an empty list of jobs. Add a test to make sure that is the case.

This re-land the change in 7ba9d48 with
compatibility fixes and additional tests.

The fix makes sure the skipped jobs are returned in the planning as well for
compatibility reasons. Not after this change:

For the build systems that use swift-driver that doesn't support
incremental build (swiftpm), asking for an incremental planning will
still get the full list of jobs, but the skipped jobs might not be batched.
The planned jobs are still complete, but might not be optimally batched.

For the build systems that supports incremental build (swift-build), the
build planning is extracted from incremental state directly. But note
swift-driver lazily wrapped the compile jobs in Executor so swift-driver
can't return an empty list of jobs. Add a test to make sure that is the
case.
@cachemeifyoucan cachemeifyoucan requested review from owenv and artemcm March 4, 2025 22:05
@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please test

@cachemeifyoucan
Copy link
Contributor Author

Alternatives are:

  • We are also batching the skipped job and return
  • We keep the original logics but be very careful when to compute hash key. Assume the build system that supports caching also do the incremental build correctly by reading from incremental states, if there is an incremental state, don't compute hash for returned job list.

@cachemeifyoucan
Copy link
Contributor Author

@artemcm @owenv any opinion on this change?

@cachemeifyoucan cachemeifyoucan merged commit a161856 into swiftlang:main Mar 7, 2025
3 checks passed
cachemeifyoucan added a commit to cachemeifyoucan/swift-driver that referenced this pull request Mar 26, 2025
This fixes a fallout from swiftlang#1829 that skipped non-compile jobs are not
returned from `planBuild()` during incremental build if there are no
compile jobs are being schedule from the first place. This can result into
an empty list of job being returned, and `swift-build` will treat that
as an error.

Now the skipped non-compile jobs are properly tracked and returned.

rdar://147874145
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