-
-
Notifications
You must be signed in to change notification settings - Fork 264
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
lock create
with "large" set of dependencies spends 95+% of time in sequential pip download
#2036
Comments
I see you added logging, but the parallelism structure and limitations are "well known" already ... to those in the know. With these constraints:
Really, that's it. With that in mind, PEX does:
The 1st step is not parallelizable and it's also the 1st and ~only step creating a lock file. To parallelize resolves means parallelizing the Pip resolve process itself upstream or else going back to bespoke. |
@huonw I'm not sure what you're gunning for here. If it's faster locks after the 1st, there is a known unimplemented solution for that, which is to use the current lock to create a venv and then run the |
Would a |
Let's just find out. @huonw can you provide a real before / after dual set of input requirements? |
Ok, I contrived an example. In short, lock update is exactly the same speed as lock re-creation given the same PEX_ROOT cache baseline; so no cheap win there: Given: That diff is: $ diff -u requirements.txt requirements2.txt
--- requirements.txt 2023-01-13 08:16:34.195292265 -0800
+++ requirements2.txt 2023-01-13 08:18:43.755243397 -0800
@@ -14,7 +14,7 @@
cffi==1.15.1
chardet==5.1.0
charset-normalizer==2.1.1
-click<9
+click<=8
colorama==0.4.6
cryptography==39.0.0
decorator==5.1.1 N.B.: I have to use a downgrade for this example since
Cache from initial lock still laying there when 2nd lock is run:
Lock update instead of re-lock with new requirements:
|
This adds logging for `pip` invocations, to make it easier to understand how pex is calling `pip`, such as when a single invocation is taking a long time (#2036). This mirrors similar logging for Python invocations: https://github.com/pantsbuild/pex/blob/1f8e25a714e52c45a50a6d6ab2ee7cb9e21a92bb/pex/interpreter.py#L713-L716
Alright, I've labelled this as a question and I'll be aiming to close as answered by the end of the day. That close pends reviving some old tests of incremental resolve (the venv trick mentioned above) to see how fast that is / what work is involved in implementing it for real. |
Thanks for waiting over the weekend, and for providing all that context. (And, of course, thank you for maintaining PEX!) In summary, feel free to close this as "won't fix" (and/or an upstream issue). I understand the design constraints better now!
I was not in the know, and I imagine many others also are not. Thanks for merging the logging, hopefully that'll save the next person in a similar situation to me the few hours I took to narrow down what was going on.
Yep, that makes sense. That's key to my understanding. In this case, there's an 'extra' bonus of the input requirements being the full transitive set of deps, locked to exact versions, so, to a naive rube like me, there's a chance no resolution is required, just finding the artefacts, which seems like it could be done in parallel, hence filing an issue (although I maybe didn't emphasise that extra constraint as much as I could've). But, I totally understand not adding special code paths for niche use-cases, given the discussion here. (We're in this circumstance in our repo: we have pants/pex create a lock file from a set of requirements exported from poetry, because poetry gives us better control around adding deps without upgrading the world, e.g. pantsbuild/pants#15704.)
Yeah, we're almost always adding/removing/adjusting dependencies within an existing lockfile, rather than creating new lockfiles. That said, for us, even more important than optimisations in PEX is being able to do away with Poetry and use only pants+PEX (using both Poetry and PEX means we pay for two resolves when changing any deps, so in addition to being more convenient, using just PEX will be 2+x faster for us, automatically). As mentioned above, one blocker for this is pantsbuild/pants#15704, so that we can upgrade existing dependencies and add new ones, without upgrading everything else as a side effect. In addition (just so that you're not over indexing on the value of this discussion), we're only likely to benefit from optimisations if they're hooked into Pant's
The most recent security update we did was bumping --- requirements-after.txt 2023-01-16 12:19:08.000000000 +1100
+++ requirements-before.txt 2023-01-16 12:19:24.000000000 +1100
@@ -12,3 +12,3 @@
cachetools==5.2.1
-certifi==2022.12.7
+certifi==2021.10.8
cffi==1.15.1 Running the same commands as you, gives similar results, for doing a full rm -rf ~/.pex && time pex3 lock create --style universal --resolver-version pip-2020-resolver --pip-version 22.3 --target-system linux --target-system mac --interpreter-constraint "==3.10.*" -r requirements-before.txt --indent 2 -o lock.jsoon
#> 21.77s user 5.99s system 50% cpu 55.085 total
time pex3 lock create --style universal --resolver-version pip-2020-resolver --pip-version 22.3 --target-system linux --target-system mac --interpreter-constraint "==3.10.*" -r requirements-after.txt --indent 2 -o lock2.json
#> 13.33s user 1.37s system 83% cpu 17.594 total As I think you were implying in your note, doing this sort of upgrade via
But, that's probably not related to the performance question here. That said, if I'm understanding this correctly, we'd preferably still want to be able to do an "minimal update" when we replace constraints. For example, a concrete instance of this that we might be doing in the near future would be upgrading |
Ok, it sounds like ~all your problems are with Pants at the moment and you'll benefit from incremental PEX locking with the venv trick when it comes. As far as update not working with pins, that's a bit of an artificial pickle you're in using poetry + Pants / Pex. Presumably, if just using Poetry or just using Pants / Pex you would not use all input pins and let locking do its job (for example allowing patch or minor to float high in your input requirements in many cases for well-behaved libraries). In that case, |
Yeah, a bit naive. For one there would need to be a signal (flag) coming from Pants to Pex to say |
I'll still leave this open until I finish off here with the incremental resolve venv trick numbers. I went as far as to realize the trick was not, in fact, create a venv from the existing lock, then run Timings coming ~tomorrow. |
Ok, and the incremental timings. The raw work:
So that's roughly 8s for the real-world use case:
That's fairly significant and worth pursuing all other things being equal. On quick inspection, the Pip log output from the I'll close this as answered and link an issue tracking implementation of incremental resolves using the lock -> venv -> pip install trick. |
Ok, the parallelization If I end up devoting time to these it will definitely be the incremental lock resolves of #2044 1st. That will both help the |
That sounds good. Thanks for working through this discussion! |
In our (pants-using) repo, we have 128 dependencies, which leads to
./pants generate-lockfiles
taking a long time, due to thepex3 lock create
invocation. It seems this invocation is slow becausepip download ...
checkshttps://pypi.org/simple/...
individually, for each package, sequentially (pypa/pip#825).I notice
pex
takes a--jobs
argument to parallelise some things, but there's only onepip download
invocation, so there's currently no parallelism. Can pex parallelise this? (Potentially only with certain options, like--intransitive
?)I've created an example a set of 88 requirements, reduced from the top 100 downloaded packages on PyPI (from https://hugovk.github.io/top-pypi-packages/, not that it really matters):
click for requirements.txt
I added a little bit of extra logging of pip invocations (#2035), and then ran:
Output (with my spaces and commentary in
# CAPS
)Based on the
ts
-inserted timestamps, the whole invocation takes ~14.4s, and thepip download
invocation takes 13.8.The pip log seems to contain sequential downloads:
The text was updated successfully, but these errors were encountered: