https://pantsbuild.org/ logo
Join Slack
Powered by
reviving a previous discussion about the caching o...
# development
w
reviving a previous discussion about the caching of interpreter lookups for python
đź‘Ť 1
we currently try not to, but there was a bug in UUIDRequest (pr coming soon) that was breaking 
--test-force
fixing the bug in UUIDRequest (would like to port that into a native implementation so that it is less error prone) adds ~1.2seconds to a noop 
test
run (from 1.2 to 2.4)
đź‘Ť 1
the 1.2 seconds is in re-running tool discovery per-run (https://gist.github.com/stuhood/5152908bd9953424d6fd2356010f13de)
while it is technically correct to do (until https://github.com/pantsbuild/pants/issues/10769 i think?)… that’s quite a cost to pay.
cc @enough-analyst-54434
h
I wonder if we do want to make it something manual..like a 
./pants invalidate-binaries
I originally thought no way to manual. It's way too error prone and easy to not know how to do. But the reality is that it seems very rare for something like /usr/bin/tar to change. Especially in CI, it's very rare to change. Even if we can lower the cost, doing it automatically will still add overhead
w
right now, a manual step would be to kill 
pantsd
or edit 
pants.toml
to force a restart
h
Yeah, but I'm saying we never re-search the binaries unless a) the discovery process failed, or b) the user runs a command. John and my original intent was to always automatically rerun every pantsd session. Even if we can lower that cost, it's still expensive to rerun
h
Yeah, that is too high a time price to pay for hypothetical correctness
h
Well, it is important for correctness. If you install a new interpreter with Pyenv, or upgrade your OS, for example, pants will not detect the change until you run a manual command. That's really easy to forget and a bad UX But I'm suspecting the even worse UX is for pants to waste a bunch of time doing something that 99% of the time is irrelevant
đź‘Ť 2
w
or upgrade your OS
that is a restart, so pantsd won’t survive it, but yes to the other thing
h
Right, I am saying that even the "per pantsd session" invalidation might be too costly and we should never automatically invalidate
w
i’m not 100% sure that we’re all using 
Session
the same way. a 
Session
is a single run of pants
there isn’t a formal name for “the span of time that 
pantsd
is up”, afaik
h
We probably aren't. The current behavior for discovery of binaries should be "per pantsd lifetime". Test force should be "per pants run"
w
got it. yea, that is not happening. we’re re-running both per-
Session
.
i’ll see if i can adjust https://github.com/pantsbuild/pants/pull/11288 to do that without going deeper in this PR.
đź‘Ť 1
h
I believe this all once worked, when John originally added it. This sounds like a regression
đź‘Ť 1
w
my hunch is that it worked for either interpreter discovery or for 
--test-force
, but not both at the same time.
h
how hard is https://github.com/pantsbuild/pants/issues/10769 to do? if we can get that in quickly, and that solves the problem conceptually correctly, then it doesn't matter much which decision we make with the --test-force PR
w
i started the thread thinking that we currently wanted to re-run discovery for every pants run. if that’s not the case, then this isn’t that bad (that behavior was working, so it just needs to be preserved in #11288)
h
ideally, we would be able to determine if the binary changed under us very quickly, so we could in fact run that check on every pants run
w
it’s harder than you might think. take a look at the gist to see how much might change under us: https://gist.github.com/stuhood/5152908bd9953424d6fd2356010f13de
đź‘Ť 1
h
ideally, we would be able to determine if the binary changed under us very quickly, so we could in fact run that check on every pants run
I disagree with every run. It rarely changes, and doesn't seem too offensive to me to have to restart pantsd if you install a new interpreter. But, would be good to make it easier to force restart pantsd
h
I think the concern is more not realizing that you have to restart pantsd
I agree that tool changes should happen rarely enough that it's not a problem if there's a manual step involved
w
@hundreds-father-404: to be clear, i agree that we should fix #10769. i just don’t think it’s urgent.
h
but yeah I would say that for right now, it's better to be technically incorrect in a way that causes silent failures occasionally than waste 1.2 seconds every time. it sounds like stu's PR as it currently stands ends up doing the latter
w
ok, i started this thread on a false premise: let's close it out and start over for further discussion: sorry!
Open in Slack
PreviousNext
Powered by