Shortcircuit local distribution source subsetting if there are no dists. #14564
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| @@ -178,6 +178,12 @@ async def build_local_dists( | |||
| ), | |||
| ) | |||
|
|
|||
| if not wheels: | |||
There was a problem hiding this comment.
Should we also shortcircuit at line 146? Other than an extra 2 lines of code, I see no reason not to.
Also I'm not sure this informative comment is necessary - short circuiting usually seems like the right thing to do!
There was a problem hiding this comment.
Should we also shortcircuit at line 146? Other than an extra 2 lines of code, I see no reason not to.
That would require adjusting the signature of LocalDistsPex to make the pex optional, which affects a lot of callsites. I started down that path, but shortcircuiting here instead allowed for the null-object pattern.
Also I'm not sure this informative comment is necessary - short circuiting usually seems like the right thing to do!
Yeaaaa. @benjyw and I both felt that landing this was punting on determining what the actual performance issue was in the trailing code: he's following up on our best guess there. If he determines why then he can delete the comment perhaps, but in the meantime let's leave it.
…uild#14564) pantsbuild#14551 improved the performance of local dist building when local distributions are actually present. But there are cases (which @benjyw is pursuing) where the `@rule` takes a long time to run, even when no dists are actually present. This is likely to do with the source subtraction: either the calculation of the subset paths, or the execution of `DigestSubset`. [ci skip-rust] [ci skip-build-wheels]
…pick of #14564) (#14566) #14551 improved the performance of local dist building when local distributions are actually present. But there are cases (which @benjyw is pursuing) where the `@rule` takes a long time to run, even when no dists are actually present. This is likely to do with the source subtraction: either the calculation of the subset paths, or the execution of `DigestSubset`. [ci skip-rust] [ci skip-build-wheels]
…uild#14564) pantsbuild#14551 improved the performance of local dist building when local distributions are actually present. But there are cases (which @benjyw is pursuing) where the `@rule` takes a long time to run, even when no dists are actually present. This is likely to do with the source subtraction: either the calculation of the subset paths, or the execution of `DigestSubset`. [ci skip-rust] [ci skip-build-wheels]
#14551 improved the performance of local dist building when local distributions are actually present. But there are cases (which @benjyw is pursuing) where the
@ruletakes a long time to run, even when no dists are actually present. This is likely to do with the source subtraction: either the calculation of the subset paths, or the execution ofDigestSubset.[ci skip-rust]
[ci skip-build-wheels]