Skip to content
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

py_binary launch performance problem on windows #2019

Open
peakschris opened this issue Jun 27, 2024 · 9 comments
Open

py_binary launch performance problem on windows #2019

peakschris opened this issue Jun 27, 2024 · 9 comments

Comments

@peakschris
Copy link

On windows, with rules_python as module and system interpreter and a simple script:

> timecmd bazel run //:myscript -- --help
command took 5.6s

> timecmd python myscript.py --help
command took 0.5s

Due to the 10x overhead, for such a regularly run tool, users are refusing to run via bazel. I'm guessing the overhead is unzipping, but am not sure. We would prefer to run without zipping if zipping has a 10x overhead.

Full output from bazel run:

INFO: Analyzed target //:myscript (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:myscript up-to-date:
  bazel-bin/myscript.zip
  bazel-bin/myscript.exe
INFO: Elapsed time: 0.494s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/myscript.exe --help
Usage: <snip>
command took 5.6s

Relevant options:

.bazelrc

startup --windows_enable_symlinks
common --noenable_runfiles
common --nolegacy_external_runfiles

Module.bazel

bazel_dep(name = "rules_python", version = "0.32.2")
# use system python for performance
register_toolchains("@bazel_tools//tools/python:autodetecting_toolchain")

🐞 bug report

Affected Rule

py_binary

Is this a regression?

No

🔬 Minimal Reproduction

I can create this if it would help

🌍 Your Environment

Operating System:

  
Windows
  

Output of bazel version:

  
bazel 7.2.1
  

Rules_python version:

  
0.32.2
  

Anything else relevant?

@aignas
Copy link
Collaborator

aignas commented Jun 27, 2024

I am wondering if bazelbuild/bazel#22865
is related. Could you please try if nondblzmod is the same overhead?

Could you also check if precompiling py files to pyc helps (that is a new feature in 0.32.2)?

Other than that, #1653 could be also related.

@peakschris
Copy link
Author

Ah, good thought! bazelbuild/bazel#22865 was opened by me, so I have the fix. Sadly the fixed bazel doesn't resolve this - launch time still 5+s

Just to confirm that I have the fix, here are the files in my external dir:
image

I don't think that #1653 is related because I am using system interpreter, and I have disabled legacy_external_runfiles

I can't understand from the docs what I should change in my py_binary target to enable precompiling. Could you advise?

Thanks, Chris

@peakschris
Copy link
Author

Ok, I updated from 0.32.2 to 0.33.2, and the precompiling option started working. But unfortunately no improvement in speed:

INFO: Elapsed time: 0.377s
bazel run //:myscript --@rules_python//python/config_settings:precompile=enabled -- --help
command took 5.61s

@peakschris
Copy link
Author

Another datapoint: Using 7z to recursively extract the zip created by py_binary takes 0.64s

@aignas
Copy link
Collaborator

aignas commented Jun 28, 2024

What is the performance if you launch the the script directly? I.e. bazel build + executing it instead of bazel run?

@rickeylev
Copy link
Collaborator

The main expensive operation that I can think of would be unzipping the binary. By default, a windows build creates a small native exe and a zip of the python program. All the exe really does is call python foo.zip. This lands in foo.zip/__main__.py, which then unzips itself, does some setup, then re-execs with the desired interpreter and actual main py file.

To check this, you can just unzip the myscript.zip, and run python __main__.py. After unzipping, you can also get a size of how big and how many files there are. The bigger and more of them, the more expensive unzipping will be.

If that is the cause, you can try explicitly passing --build_python_zip=false (for windows, it defaults to true, unlike other platforms). It may or may not work because (1) we fixed a bug with this case, not sure if its released yet or not, and (2) i think it requires runfiles or symlinks enabled; not really sure (I don't do much windows)

HTH

@peakschris
Copy link
Author

Thank you, this is very helpful. The measurements below are with rules_python 0.34.

So with a larger python project, myscript.zip is 34MB - 95% python, 4.9% pip modules, 0.1% script code. Time to get help message is:

* bazel run myscript: 19.6s
* 7z x myscript.zip: 8.1s
* bazel --windows_enable_symlinks run --build_python_zip=false --enable_runfiles myscript: 2s [inc 0.5s bazel up-to-date check]
* python myscript.py: 0.26s [in venv, previously setup]

rules_python and bazel is still slower, but much closer.

If I try --build_python_zip=false without --enable_runfiles, I get this error. Seems like whatever is calling the bootstrap template is not resolving the script location using the manifest. Is this the issue you mention?

INFO: Running command line: bazel-bin/external/rules_myscript~/tools/myscript.exe --help
Traceback (most recent call last):
  File "<snip>\execroot\_main\bazel-out\x64_windows-fastbuild\bin\external\rules_myscript~\tools\myscript", line 573, in <module>
    Main()
  File "<snip>\execroot\_main\bazel-out\x64_windows-fastbuild\bin\external\rules_myscript~\tools\myscript", line 497, in Main
    assert os.path.exists(main_filename), \
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Cannot exec() '<snip>\\bazel-out\\x64_windows-fastbuild\\bin\\external\\rules_myscript~\\tools\\write_sbom.exe.runfiles\\_main\\..\\rules_myscript~\\tools\\myscript.py': file not found.
command took 0:0:2.63 (2.63s total)

It would be preferable if rules_python could perform on windows with OOTB flags.

I can think of the following possible improvements:

  1. Support nozip mode without runfiles, by adding/fixing manifest lookup code (if fix is not already in pipeline), and then make it the default.
  2. Unzip only once, not on every invocation of the script
  3. Unzip the python interpreter only once, to a common location
  4. If 7zip is present (it's installed by default on recent windows), use it to unzip instead of python. This is the lowest priority fix, as whilst it is 2x faster, it's still too slow.

@peakschris
Copy link
Author

I've just checked with rules_python@HEAD, nothing in my message above is fixed/improved there vs 0.34

@aignas
Copy link
Collaborator

aignas commented Jul 14, 2024

Commenting on feasibility of the suggested fixes:

  1. Working well without runfiles support might bring too much Windows specific code that might become hard to test. I think a requirement for enabling runfiles is reasonable. Is there any downside on using it on Windows?
  2. Unzipping once might be difficult to implement reliably (how you know that you should reuse the file instead of overwrite it), so it would be better if somebody came along and attempted to get it running with runfiles and without zip support.
  3. Depending on 7z if it exists in the system is a no go in my opinion, because the builds and the runtime behaviour should be hermetic. If we wanted, maybe we could bundle an unzipping program written in a low level language, but I am not sure what would be the return on investment here.

These are my opinions and maybe sometimes has more things to share here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants