Skip to content
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

[internal] Structure sharing for CoarsenedTargets #13087

Merged

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Oct 2, 2021

Profiles in example Java repositories showed (unsurprisingly) that recursively re-computing CoarsenedTargets (each of which did a transitive walk, and then discarded all but the CoarsenedTargets for the input root) was a bottleneck.

Unlike Targets, CoarsenedTargets are by definition acyclic, and so can share structure. This change converts CoarsenedTarget to a transitive DAG, and CoarsenedTargets to containing only the roots of the walk.

Computing CoarsenedTargets no longer dominates compilation in example repos.

[ci skip-rust]

[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
Comment on lines +180 to +181
# If the component has no sources, it is acting as an alias for its dependencies: return
# their merged classpaths.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this represent? An explicit dep on a java_sources target generator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any target type with no matching sources: so yea, java_sources, lockfile targets, etc.

@stuhood stuhood merged commit 04360e9 into pantsbuild:main Oct 5, 2021
@stuhood stuhood deleted the stuhood/coarsened-target-structure-shares branch October 5, 2021 04:07
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this pull request Oct 5, 2021
Profiles in example Java repositories showed (unsurprisingly) that recursively re-computing `CoarsenedTargets` (each of which did a transitive walk, and then discarded all but the `CoarsenedTarget`s for the input root) was a bottleneck.

Unlike `Targets`, `CoarsenedTargets` are by definition acyclic, and so can share structure. This change converts `CoarsenedTarget` to a transitive DAG, and `CoarsenedTargets` to containing only the roots of the walk.

Computing `CoarsenedTargets` no longer dominates compilation in example repos.
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
@stuhood stuhood mentioned this pull request Oct 11, 2021
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