<@U0N6C2Q9F>: good morning!
# development
w
@fast-nail-55400: good morning!
i made some progress on the perf triage yesterday, and one of the two issues that jumped out was https://github.com/pantsbuild/pants/issues/13787
it’s significant enough that it will probably muddy the water for further profiling
and it’s a prerequisite for https://github.com/pantsbuild/pants/issues/12716 (although the profiling didn’t immediately show that #12716 will be necessary)
if you’re interested in picking that one up after https://github.com/pantsbuild/pants/pull/13777, i can take on the generate-lockfile merging work
f
and it’s a prerequisite for https://github.com/pantsbuild/pants/issues/12716
do you mean the opposite? 12716 is a prereq for 13787?
since 13787 would make use of whatever is implemented for 12716
w
“very related” perhaps
they’ll both use the same mechanism of splitting the
input_digest
i think, such that the actual digest used for remexec becomes a computed value
(currently the
use_nailgun
digest is just ignored for digesting the action, because it is known to be included in the
input_digest
)
@fast-nail-55400: to be clear though: the triage didn’t show that we need 12716 yet (although i think that we will for larger repositories)
but 13787 is a 200% overhead for source analysis, and muddies the water around how much io is currently necessary
f
https://github.com/pantsbuild/pants/pull/13271 already some related code for doing the merging in remote execution
(it is the prototype PR for 12716)
it is blocked on deciding whether the API changes to
Process
are what we want
it’s just an API change, no implementation change there
w
ok. the gist of https://github.com/pantsbuild/pants/issues/13787 is “we need to make a change that actually splits out digests”. so in that regard: yes, we need an API change to Process
i’ll take a look at #13271, but the question above stands: are you interested in working on 13787 if i pick up the generate-lockfiles merging?
commented on #13271: the high level API seems fine, but the actual merging of the digests (which is the bit shared with #13787) would need changes
@fast-nail-55400: so… do you want this one, or should i…?
f
I’m still working on https://github.com/pantsbuild/pants/pull/13777 and then there is an additional change needed for Go file embedding that I need to make after
(to Go code but only after Eric completes some prework on the file embedding)
so feel free to take whatever is open, I’m not likely to get to 13788 today
w
ok
thanks!