https://pantsbuild.org/ logo
#general
Title
# general
a

ancient-france-42909

08/18/2022, 11:37 PM
Splitting this into a new thread:
generate-lockfiles
takes 5 minutes per ‘thing’, how can I find out why?
Copy code
$ ls ~/.cache/pants/
lmdb_store	named_caches	setup
w

witty-crayon-22786

08/18/2022, 11:46 PM
what does the UI render while this is going on? assuming that the majority of the time is spent in a PEX subprocess, you’d need to profile the subprocess.
a

ancient-france-42909

08/18/2022, 11:47 PM
It’s slightly faster on 2.13.
👍 1
w

witty-crayon-22786

08/18/2022, 11:48 PM
good…
a

ancient-france-42909

08/18/2022, 11:48 PM
So from 10+ minutes to:
Copy code
real    0m38.479s
w

witty-crayon-22786

08/18/2022, 11:48 PM
ah, good.
if it’s still slower than you’re expecting, the next step would be to re-run with
--no-local-cache --ldebug
, capture the “spawning local process” DEBUG line for the relevant process, and then report a github.com/pantsbuild/pex issue with the content
a

ancient-france-42909

08/18/2022, 11:50 PM
I ran
./pants check ::
though, it’s been resolving coaresned targets for 40 seconds, then got warnings about
interpreter_constraints
and then no checks, heh
w

witty-crayon-22786

08/18/2022, 11:51 PM
that would be a different topic probably.
a

ancient-france-42909

08/18/2022, 11:51 PM
almost all our dependencies are pinned to a single version (99%), so not sure what that’s doing
ah, the interpreter constraints thing is an error now
yeah
and I have to get up in 5 hours 😄
w

witty-crayon-22786

08/18/2022, 11:52 PM
the interpreter constraints thing is an error now
yea… that change is actually intended to improve performance
a

ancient-france-42909

08/19/2022, 12:04 AM
yeah, we just finished migrating from a mixed environment, nobody ever bothered to fix the
^3.7
vs
>3.7
stuff, I just removed everything basically 🙂
👍 1
Okay, so even with 2.13.0rc1, running generate lockfiles is slow. It was fast once last night, now it’s again bad.
Copy code
$ ./pants generate-lockfiles --resolve=pytest
⠐ 480.20s Resolve transitive targets
⠐ 480.20s Resolve transitive targets
⠐ 469.92s Resolve transitive targets
⠐ 470.12s Resolve transitive targets
⠐ 465.09s Resolve transitive targets
And it’s using 5GB of memory (or more)
h

hundreds-father-404

08/19/2022, 4:09 PM
Hello. Hm, that's helpful. That is not the Pex subprocess. That is Pants itself before passing things to Pex, in figuring out your dependency graph. We would never expect it to take this long 😱 What was your performance like in Pants 2.6(?) when you do
./pants dependencies --transitive ::
? (To help us figure out if this is a regression)
a

ancient-france-42909

08/19/2022, 4:10 PM
I’m gonna try to wait for this to finish before I switch branches, sunk cost fallacy, I already waited 15 minutes… 😄
h

hundreds-father-404

08/19/2022, 4:14 PM
That definitely sounds like something is broken, like an infinite loop or pathological code path. Which we'd love to figure out, thank you for reporting this In the meantime to trying out 2.6, I'm curious: • how many files are in your repo? • Are you generally using one BUILD file per directory (what
tailor
does), or fewer BUILD files with
**
globs in the
sources
field? • are there import cycles between your files? • what does
./pants --concurrent roots | wc -l
in another terminal say? None of those would explain 15 minutes...but give us some info on the shape of your repository.
a

ancient-france-42909

08/19/2022, 4:14 PM
It’s not an infinite loop, it di finish.
h

hundreds-father-404

08/19/2022, 4:15 PM
Other questions that could be helpful: • what operating system you're using • how much RAM your machine has
a

ancient-france-42909

08/19/2022, 4:18 PM
23738 files (not all code, but the overwhelming majority), 262 BUILD files, though not one per directory, one per app/lib. I imagine there might be some, but I wouldn’t know offhand. 272 roots. It’s a 2019 mac, i7 I think, with 16GB of memory (I know 😞 ).
👍 1
2.6 dependencies took 54 seconds
h

hundreds-father-404

08/19/2022, 4:22 PM
2.6 dependencies took 54 seconds
Certainly slower than we'd like—and we continue to make performance optimizations—but yeah, that's within the upper range I would expect for a repo that size. So, it sounds like somewhere between 2.6 and 2.13 there was a change that made this pathologically slow for you. It sounds like you were upgrading one version at a time. When do you first remember noticing how slow this was? I think there could be two approaches to figuring this out: 1. Bisect when things slowed down for you, or 2. use profiling to figure out on 2.13 where the slowdown is happening
a

ancient-france-42909

08/19/2022, 4:23 PM
I, unfortunately, didn’t commit anything between 2.6 and I think 2.10 😞 And even then, some things were horribly broken still.
h

hundreds-father-404

08/19/2022, 4:23 PM
some things were horribly broken still.
Meaning this performance slowdown, or other things?
a

ancient-france-42909

08/19/2022, 4:23 PM
But, as I said, last night when I ran it, on 2.13, it was fast
Oh, no, I mean the BUILD files were broken 🙂
So, running
./pants dependencies --transitive ::
on 2.13 took 2m23s
h

hundreds-father-404

08/19/2022, 4:29 PM
Okay, much much slower than we want for sure, and that's a bad regression from 2.6. But at least it terminates.. To clarify, is the extremely slow
generate-lockfiles
command on 2.13 also?
./pants dependencies --transitive ::
should trigger the same "Resolving targets" rule as
generate-lockfiles
was doing
a

ancient-france-42909

08/19/2022, 4:30 PM
I just ran it again (
./pants generate-lockfiles --resolve=pytest
), it’s been running for mroe than 2.5 minutes and doesn’t seem like it’s finishing. And, yeah, 2.13
h

hundreds-father-404

08/19/2022, 4:30 PM
Still stuck on the "Resolving transitive targets" step?
a

ancient-france-42909

08/19/2022, 4:30 PM
I wouldn’t say stuck, sometimes the timers do appear stuck, but not for very long.
w

witty-crayon-22786

08/19/2022, 4:31 PM
@hundreds-father-404: if there are multiple entries for
Resolve transitive targets
, then it’s related to https://github.com/pantsbuild/pants/issues/11270 … but that should no longer be relevant under
generate-lockfiles
, I don’t think?
👍 1
a

ancient-france-42909

08/19/2022, 4:31 PM
Screen Recording 2022-08-19 at 19.30.52.mov
👍 1
w

witty-crayon-22786

08/19/2022, 4:31 PM
would want to find where we’re making multiple TT calls for
generate-lockfiles
a

ancient-france-42909

08/19/2022, 4:32 PM
How can I help?
❤️ 1
w

witty-crayon-22786

08/19/2022, 4:32 PM
@ancient-france-42909: which backends do you have enabled?
h

hundreds-father-404

08/19/2022, 4:33 PM
would want to find where we’re making multiple TT calls for generate-lockfiles
@witty-crayon-22786 I suspect this has pathological behavior in their repo:
constraints = await _pytest_interpreter_constraints(python_setup)
Copy code
all_tgts = await Get(AllTargets, AllTargetsRequest())
    transitive_targets_per_test = await MultiGet(
        Get(TransitiveTargets, TransitiveTargetsRequest([tgt.address]))
        for tgt in all_tgts
        if PythonTestFieldSet.is_applicable(tgt)
    )
But I don't understand why
dependencies --transitive ::
would be faster; iiuc it does the same type of
MultiGet
a

ancient-france-42909

08/19/2022, 4:33 PM
Copy code
'pants.backend.python',
    "pants.backend.docker",
    "pants.backend.plugin_development",
    'pants.backend.python.lint.bandit',
    'pants.backend.python.lint.black',
    'pants.backend.python.lint.flake8',
    'pants.backend.python.lint.isort',
    'pants.backend.python.lint.pylint',
    "pants.backend.python.typecheck.mypy",
    "pants.backend.python.lint.docformatter",
    "pants.backend.python.mixed_interpreter_constraints",
And a custom one, which reminds me, I might’ve had it off last night when I was trying.
w

witty-crayon-22786

08/19/2022, 4:35 PM
@hundreds-father-404: could be, yea. could maybe try resolve-by-resolve.
h

hundreds-father-404

08/19/2022, 4:37 PM
Stu, would it make sense to ask for
-ldebug
or anything like that to confirm where the rule is getting stuck?
a

ancient-france-42909

08/19/2022, 4:37 PM
I ran it without our plugin, it’s past 3 minutes now, so I don’t think it’s that.
h

hundreds-father-404

08/19/2022, 4:39 PM
could maybe try resolve-by-resolve.
George-Cristian, for context, I would expect
generate-lockfiles --resolve=isort
to be much faster, for example, because it does not need to consult your own code's interpreter constraints to figure out which to use for the lockfile. I suspect it's that interpreter constraints calculation for Pytest that is messing us up
a

ancient-france-42909

08/19/2022, 4:39 PM
Yes, that gets tot he generating lockfile stage instantly, 20s in total
👍 1
This is our pytest section of the config:
Copy code
[pytest]
lockfile = "pants-lockfile.pytest"
args = ["--durations=50"]
version = "pytest==7.0.1"
extra_requirements.add = [
    # improve diffs when comparing objects in tests
    "pytest-icdiff==0.5",
    # add syntax highlighting for pytest output
    "pygments==2.9.0",
    # allow to use ipdb instead of pdb in debug mode
    "ipdb==0.13.2",
    # required by the pytest configuration in `pytest.ini`
    "SQLAlchemy==1.3.0",
    # Somehow pants decides to upgrade that.
    "pyparsing==2.4.7",
]
👍 1
h

hundreds-father-404

08/19/2022, 4:40 PM
Hey @witty-crayon-22786, one idea is https://github.com/pantsbuild/pants/issues/12652. I think it would allow us to short circuit most of the graph walking (Although that's a workaround -- this should not be so pathologically slow either way)
w

witty-crayon-22786

08/19/2022, 4:41 PM
…er. wait a sec.
``` all_tgts = await Get(AllTargets, AllTargetsRequest())
transitive_targets_per_test = await MultiGet(
Get(TransitiveTargets, TransitiveTargetsRequest([tgt.address]))
for tgt in all_tgts
if PythonTestFieldSet.is_applicable(tgt)
)```
is no longer necessary after the IC deprecation, right?
h

hundreds-father-404

08/19/2022, 4:41 PM
Oh, that it can be solely looking at the "root" targets?
w

witty-crayon-22786

08/19/2022, 4:41 PM
would still want to confirm that it’s actually that one, but
👍 1
right.
h

hundreds-father-404

08/19/2022, 4:43 PM
Yeah, that sounds correct to me! We can cherry-pick that to 2.13 because I think the deprecation was complete by then for ICs?
a

ancient-france-42909

08/19/2022, 4:43 PM
I’m going to have dinner, but will be back in a bit, if you want me to try stuff
w

witty-crayon-22786

08/19/2022, 4:44 PM
@hundreds-father-404: correct: it’s an error in 2.13: https://github.com/pantsbuild/pants/pull/15531 is in there.
👍 1
h

hundreds-father-404

08/19/2022, 4:44 PM
Thank you @ancient-france-42909 💜 If the above is all true, we have a path forward and can cherry-pick to 2.13! @witty-crayon-22786 I don't see where else this could be but the already quoted lines on ICs. No need for
-ldebug
to confirm
w

witty-crayon-22786

08/19/2022, 4:45 PM
🤞
h

hundreds-father-404

08/19/2022, 5:00 PM
@ancient-france-42909 one command that would help us to confirm this fix:
./pants generate-lockfiles --resolve=black
, and make sure that
[black].interpreter_constraints
is not set. My change updates Pytest to use the same algorithm as Black was already using
Awesome, for pantsbuild/pants (relatively small repo), this change means calculating the interpreter constraints for
generate-lockfiles --resolve=pytest
goes from 4.7 seconds to 0.4 seconds 🎉
a

ancient-france-42909

08/19/2022, 5:13 PM
Black was fast.
🙌 1
h

hundreds-father-404

08/19/2022, 5:15 PM
Awesome! I will have something out soon, and then we can do a release of 2.13 this weekend
a

ancient-france-42909

08/19/2022, 8:10 PM
btw, if you want me to test it, I can do it for a while tonight, but then not sure how much time I’ll have over the weekend. I mean, it’s not an issue as long as it’s related to generate-lockfiles, since I’m not going to do that anytime soon (someone else updated pytest today 😮), just wanted to make sure it makes it into the 2.13 release 🙂
h

hundreds-father-404

08/19/2022, 8:29 PM
Thank you! This is the PR and I will cherry-pick. Have a great weekend if we don't talk between then 🙂 https://github.com/pantsbuild/pants/pull/16591
a

ancient-france-42909

08/19/2022, 8:32 PM
is there an easy way to install from a branch? I was reading the docs earlier and it seemed to suggest you can use a SHA to install from master only
h

hundreds-father-404

08/19/2022, 9:10 PM
indeed, because we only build the wheels and push to S3 on
main
but given your report that
black
was fast, I'm fairly confident this PR will fix your issue with pytest 🙂
a

ancient-france-42909

08/19/2022, 9:11 PM
Okay, thanks! Now if you can make general dep resolving faster… 😄
h

hundreds-father-404

08/20/2022, 12:28 AM
Once this is merged, you'll be able to use PANTS_SHA https://github.com/pantsbuild/pants/pull/16596 We might have me do one related performance improvement on Monday, which would mean official release on Monday or Tuesday
a

ancient-france-42909

08/23/2022, 7:14 PM
Btw, that works great. Lockfiles are generated in 39 seconds for everything, vs 15+ minutes for one (though, I think it didn’t really scale linearly, I never tried)
🤘 2
❤️ 1
h

hundreds-father-404

08/23/2022, 8:32 PM
Thanks for the update! That's great to hear 😄 I'm trying to get out the official release today, some CI issues
a

ancient-france-42909

08/23/2022, 8:59 PM
It’s okay, I don’t think I’ll be able to fix even half the issues I KNOW about this week, let alone more 🙂
2 Views