-
-
Notifications
You must be signed in to change notification settings - Fork 638
refactor: defer zip manifest building to execution phase to improve analysis phase performance #3381
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
refactor: defer zip manifest building to execution phase to improve analysis phase performance #3381
Conversation
|
As mentioned in the other PR comment: we can definitely accept PR until the questions about using an external tool are worked out. Overall, LGTM. Because of the profile results in the other thread showing format() (as called by map_zip_runfiles) was a significant chunk of the time, changing to |
py_binary and py_test rules
py_binary and py_test rules|
Ok, cleaned this up, switched it to use @tobyh-canva If you have opportunity, could you run another profile? I'm interested to see how much of the 23 seconds of format() overhead in building the path strings is gone. |
|
Running a profile right now, thanks heaps! |


When py_binary/py_test were being built, they were flattening the runfiles
depsets at analysis time in order to create the zip file mapping manifest for
their implicit zipapp outputs. This flattening was necessary because they had
to filter out the original main executable from the runfiles that didn't belong
in the zipapp. This flattening is expensive for large builds, in some cases
adding over 400 seconds of time and significant memory overhead.
To fix, have the zip file manifest use the
runfiles_with_exeobject, which isthe runfiles, but pre-filtered for the files zip building doesn't want. This
then allows passing the depsets directly to
Args.add_alland using map_eachto transform them.
Additionally, pass
runfiles.empty_filenamesusing a lambda. Accessing thatattribute implicitly flattens the runfiles.
Finally, because the original profiles indicated
str.format()was a non-trivialamount of time (46 seconds / 15% of build time), switch to using
+instead.This is a more incremental alternative to #3380 which achieves most of the
same optimization with only Starlark changes, as opposed to introducing an
external script written in C++.
Profile of a large build, which shows a Starlark CPU profile. It shows an overall build
time of 305 seconds. 46 seconds (15%) are spent in
map_zip_runfiles, half of whichis in
str.startswith()and the other half instr.format().