Require that a target's interpreter_constraints are a subset of their dependencies' (Cherry-pick of #15373)
#15407
Conversation
This file contains 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
…ir dependencies' (pantsbuild#15373) As described in pantsbuild#15241: we currently compute per-target interpreter constraints by doing per-target graph walks, which is both a scalability bottleneck (because you can almost never use the constraints directly on a target: you must compute them), and complex for users to reason about. This change adds a `ValidateDependenciesRequest` union, which allows backends to validate the computed dependencies of a target. The python backend uses validation to deprecate the condition from pantsbuild#15241. When that deprecation triggers, most/all callsites which currently use `create_from_compatibility_fields`, `create_from_targets`, or the new `compute_from_targets` can instead directly consume the ICs of a root target in the graph. This change also (temporarily) adds an `InterpreterConstraints.compute_from_targets` method which computes (in transitive-dependency-linear time) that the dependencies of a target have a superset of its own `interpreter_constraints`. This method allows us to (again, temporarily: see above) apply the optimization of avoiding set merging if targets are already valid. Reduces the runtime of un-memoized-but-cached `./pants check ::` by 10%. Fixes pantsbuild#15241, fixes pantsbuild#15301, fixes pantsbuild#11072. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
approved these changes
May 11, 2022
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.
As described in #15241: we currently compute per-target interpreter constraints by doing per-target graph walks, which is both a scalability bottleneck (because you can almost never use the constraints directly on a target: you must compute them), and complex for users to reason about.
This change adds a
ValidateDependenciesRequestunion, which allows backends to validate the computed dependencies of a target. The python backend uses validation to deprecate the condition from #15241. When that deprecation triggers, most/all callsites which currently usecreate_from_compatibility_fields,create_from_targets, or the newcompute_from_targetscan instead directly consume the ICs of a root target in the graph.This change also (temporarily) adds an
InterpreterConstraints.compute_from_targetsmethod which computes (in transitive-dependency-linear time) that the dependencies of a target have a superset of its owninterpreter_constraints. This method allows us to (again, temporarily: see above) apply the optimization of avoiding set merging if targets are already valid.Reduces the runtime of un-memoized-but-cached
./pants check ::by 10%.Fixes #15241, fixes #15301, fixes #11072.