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

Changed spawn() and the rerun script to call into rerun_bindings (12x startup time improvement) #4053

Merged
merged 3 commits into from
Oct 30, 2023

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Oct 28, 2023

What

Python package can include so-called "scripts" definitiosn, which are recognised by pip, pipx, etc. When they exist, a stub script is created upon package installation which calls into the defined python function. We use this to map rerun to launch the viewer from the binary embedded in our wheels.

So far, this was done as follows (from pyproject.toml):

[project.scripts]
rerun = "rerun.__main__:main"

The drawback of this is that the entire rerun SDK package was loaded (including numpy, Pillow, etc.), incurring a ~300+ms startup time.

This PR changes it to:

[project.scripts]
rerun = "rerun_bindings:main"

This drastically reduces the loading time, as the rerun python package is no longer loaded. The signature of our binding's main function needed to be changed: it now internally reads from sys.argv instead of accepting an argument.

This PR also changes spawn() to call directly into the bindings, with the same loading time benefit.

Note: This PR improves startup time when running rerun in a venv (with the SDK installed), or when installed via pipx. The startup time of python -m rerun is not improved, as this still involves loading the rerun python package.

Benchmark

❯ cat `which rerun`
   1   │ #!/Users/hhip/src/rerun/venv/bin/python
   2   │ # -*- coding: utf-8 -*-
   3   │ import re
   4   │ import sys
   5   │ from rerun_bindings import main
   6   │ if __name__ == '__main__':
   7   │     sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
   8   │     sys.exit(main())

❯ cat `which rerun_0.9.1`
   1   │ #!/Users/hhip/.local/pipx/venvs/rerun-sdk-0-9-1/bin/python
   2   │ # -*- coding: utf-8 -*-
   3   │ import re
   4   │ import sys
   5   │ from rerun.__main__ import main
   6   │ if __name__ == '__main__':
   7   │     sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
   8   │     sys.exit(main())

❯ hyperfine --warmup 5  "rerun --version" "rerun_0.9.1 --version"
Benchmark 1: rerun --version
  Time (mean ± σ):      34.8 ms ±   1.7 ms    [User: 21.8 ms, System: 11.1 ms]
  Range (min … max):    32.5 ms …  41.5 ms    82 runs
 
Benchmark 2: rerun_0.9.1 --version
  Time (mean ± σ):     422.1 ms ±  52.2 ms    [User: 716.1 ms, System: 1971.6 ms]
  Range (min … max):   333.0 ms … 494.0 ms    10 runs
 
Summary
  rerun --version ran
   12.14 ± 1.62 times faster than rerun_0.9.1 --version

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

…run_bindings` (12x startup time improvement)
@abey79 abey79 added include in changelog enhancement New feature or request 🧑‍💻 dev experience developer experience (excluding CI) and removed enhancement New feature or request labels Oct 28, 2023
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Have you tested that spawn still works?

rerun_py/rerun_sdk/rerun/__main__.py Outdated Show resolved Hide resolved
rerun_py/src/python_bridge.rs Show resolved Hide resolved
@teh-cmc
Copy link
Member

teh-cmc commented Oct 30, 2023

Awesome! Have you tested that spawn still works?

Speaking of spawn, why does spawn still use python -m rerun rather than rerun? It won't benefit from the speed up if I understand correctly?

@abey79
Copy link
Member Author

abey79 commented Oct 30, 2023

Awesome! Have you tested that spawn still works?

Yes.

Speaking of spawn, why does spawn still use python -m rerun rather than rerun? It won't benefit from the speed up if I understand correctly?

Excellent point. Let me take a look at that.

@abey79
Copy link
Member Author

abey79 commented Oct 30, 2023

@teh-cmc Easy enough!

@abey79 abey79 changed the title Changed the rerun (python) script to call into rerun_bindings (12x startup time improvement) Changed spawn() and the rerun script to call into rerun_bindings (12x startup time improvement) Oct 30, 2023
@abey79 abey79 added this to the 0.10 C++ milestone Oct 30, 2023
@abey79 abey79 merged commit c8cff96 into main Oct 30, 2023
35 of 36 checks passed
@abey79 abey79 deleted the antoine/fast-startup branch October 30, 2023 08:58
emilk pushed a commit that referenced this pull request Oct 30, 2023
- Slowness was fixed by #4053

### What

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4073) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4073)
- [Docs
preview](https://rerun.io/preview/7199492ed9a4765beddd326569584f48c385e1ae/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/7199492ed9a4765beddd326569584f48c385e1ae/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants