Skip to content

Conversation

davidungar
Copy link
Contributor

@davidungar davidungar commented Jan 26, 2020

  1. Changed granularity of (fine-grained) dependency graph traversal to support tracking changes to individual node fingerprints.
  2. Changed semantics of transitive traversal to simplify meaning of a mark from "was a dependent of a cascading job" to "this node has been traversed".
  3. Adds -enable-type-fingerprints disabled by default.
  4. Changes driver handling of jobs when one job fails. Don't bother marking all unfinished jobs as cascading if they were previously thought to be. Just mark then as non-cascading-build required. On next run, if they are cascading the dependent jobs will get run.
  5. When the option above is set, the parser will hash tokens within a nominal type body or extension body into a per-body hash instead of the whole-file interface hash.
  6. Adjust tests accordingly.

-- A virtually-identical PR, 29482, has been examined by @CodaFi

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test os x platform

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test os x platform

@davidungar
Copy link
Contributor Author

@swift-ci please clean smoke test os x platform

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test os x platform

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar davidungar merged commit a2df6b9 into swiftlang:master Jan 28, 2020
@davidungar davidungar changed the title [DNM, Incremental: Type-body-fingerprints on-by-default] [Incremental] Supports per-type-body fingerprints, on-by-default Jan 28, 2020
@davezarzycki
Copy link
Contributor

davezarzycki commented Jan 28, 2020

Why MD5? Why not SHA1 or SHA256 which can be hardware accelerated on some processors? (Yes, SHA1 is slightly bigger, but you can just truncate the result if that matters.)

@CodaFi
Copy link
Contributor

CodaFi commented Jan 29, 2020

I imagine it’s because LLVM already has the facilities for this. I’d prefer we use something like 128-bit siphash but it’s certainly a decision we can revisit.

@davidungar
Copy link
Contributor Author

Yes, the use of MD5 follows what was done for the original, whole-file, interface hash.

marcrasi pushed a commit to marcrasi/swift that referenced this pull request Feb 7, 2020
[DNM, Incremental: Type-body-fingerprints on-by-default]
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