https://pantsbuild.org/ logo
Join Slack
Powered by
> ...we should probably just re-enable the pex ...
# development
h
...we should probably just re-enable the pex cache, and trust it to be concurrency safe.
We should probably take this to the new channel #performance , but we’ve been discussing this proposal at Toolchain and think it’s likely a good change to re-enable. At a minimum, re-enable when we know it’s local execution. Pex has sane caching afaict (John put a lot of sweat into ensuring that)
w
yea.
ditto 
coursier
, for example.
👍 1
it's not a universal truth, but.
h
MyPy is another example. MyPy has very sane caching that the Dropbox engineers spent 2 years working on. We’ve found that ignoring that MyPy cache results in roughly 5-10x worse performance (not formally benchmarked)
w
re-enable when we know it’s local execution
i'm not sure this differentiation is worth it... in the remote case, the cost of writing to the cache should be minimal. and depending on the remoting implementation, it might not be in docker, which would allow it to actually share caches across invokes.
👍 1
h
The only gotcha is that Google RBE is read-only. That’s why John originally turned off the Pex cache
But, maybe we could use a simple heuristic or linux command to see if the FS is read-only? Only use the cache iff it’s possible?
w
hm... read only? really?
h
(let me retrace that PR to confirm. I remember seeing it last week when I checked this out)
w
funky.
h
w
Ah, so not RBE. My understanding is that in docker, by default the homedir would be writable
But... what does "default" really mean.
h
Do you think it would be better to implement “In the Python rule, allow passing extra args to the EPR iff running locally” or “Add some mechanism in the Python rule to inspect if the FS is readable, then use that to conditionally create the EPR args”
Neither sound great, but re-enabling the Pex cache would be an enormous win for V2 performance. Resolving requirements is by far the biggest slowdown
e
Google RBE is (or was) not read only, which is bad. Toolchain's remote exec env is though mod the chroot dir.
👍 1
If we're going to support "deemed safe" caches, we really need to plumb a hole into EPRs to allow for input digests demmed safe and not part of fingerprint.
👍 1
w
well, this gets to the point i initially raised then.
we have that actually... had forgotten.
e
Ok
w
with the downside that it actually computes the whole input digest, which might not be the best for a cache.
h
unsafe_local_only_files_because_we_favor_speed_over_correctness_for_this_rule
?
😀 1
e
This will be a problem for all resolvers afaict, so a standard solution will be great.
👍 1
w
and which gets to the point i was originally making about "resolve per library".
h
running locally seems like more of a fundamental operation as far as pants is concerned, than checking if a file system is rw or not
👍 1
h
Not sure I understand "resolve per library"? Today e.g., when you run tests we resolve once for the dep closure. What are you proposing?
w
resolving once per library! literally
that would have the effect of populating a cache of artifacts at each step
which would grow as you moved "up" the graph
such that when you were building a root, most things would be in the cache.
since hitting the resolve cache is not "massively" faster than missing it (due to overheads in pip, mostly), it's unlikely to be a win without refactorings
h
By library, do you mean 3rd party distribution? Then using PEX_PATH to merge them all?
w
python_library
h
Hm, I don’t follow. What precisely do you mean by “resolve” here?
w
you would not be able to merge with pex_path, because the resolves would be different at each stage
a pex/pip resolve for the transitive dependencies of the target
Open in Slack
PreviousNext
Powered by