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

Add instrumentation support for develop #2019

Merged
merged 2 commits into from
Apr 2, 2024
Merged

Conversation

konstin
Copy link
Member

@konstin konstin commented Apr 2, 2024

The main bottleneck in the edit-debug cycle for maturin users is maturin develop. I've added tracing instrumentation and a profiling cargo profile to get information what's slow there (besides the obvious rustc/llvm performance). The instrumentation is basic and needs to be adjusted as optimizations are made.

This change allows e.g. running

NO_COLOR=1 RUST_LOG=maturin=debug cargo run --profile profiling develop --manifest-path test-crates/pyo3-mixed/Cargo.toml | grep close > log.txt

to get timing information, here for warm cache pyo3-mixed:

2024-04-02T08:20:34.576230Z  INFO run:into_build_context:resolve_cargo_metadata: maturin::project_layout: close time.busy=22.5ms time.idle=405ns
2024-04-02T08:20:34.598489Z  INFO run:into_build_context:resolve_cargo_metadata: maturin::project_layout: close time.busy=22.1ms time.idle=590ns
2024-04-02T08:20:34.625824Z  INFO run:into_build_context:check_executable: maturin::python_interpreter: close time.busy=10.6ms time.idle=903ns executable=/home/konsti/projects/maturin/.venv/bin/python
2024-04-02T08:20:34.625838Z  INFO run:into_build_context: maturin::build_options: close time.busy=72.1ms time.idle=990ns
2024-04-02T08:20:34.637709Z  INFO run:check_executable: maturin::python_interpreter: close time.busy=11.9ms time.idle=669ns executable=/home/konsti/projects/maturin/.venv/bin/python
2024-04-02T08:20:34.819501Z  INFO run:install_dependencies: maturin::develop: close time.busy=182ms time.idle=643ns
2024-04-02T08:20:34.836419Z  INFO run:build_wheels:warn_missing_py_init: maturin::compile: close time.busy=4.92ms time.idle=883ns
2024-04-02T08:20:34.841008Z  INFO run:build_wheels:write_bindings_module: maturin::module_writer: close time.busy=3.47ms time.idle=531ns
2024-04-02T08:20:34.841372Z  INFO run:build_wheels: maturin::build_context: close time.busy=21.9ms time.idle=596ns
2024-04-02T08:20:35.168696Z  INFO run:pip_install_wheel:fix_direct_url: maturin::develop: close time.busy=130ms time.idle=1.60µs wheel_filename=/tmp/.tmpPIBH5Z/pyo3_mixed-2.1.5-cp311-cp311-linux_x86_64.whl
2024-04-02T08:20:35.168708Z  INFO run:pip_install_wheel: maturin::develop: close time.busy=327ms time.idle=285ns wheel_filename=/tmp/.tmpPIBH5Z/pyo3_mixed-2.1.5-cp311-cp311-linux_x86_64.whl
2024-04-02T08:20:35.168869Z  INFO run: maturin: close time.busy=654ms time.idle=1.64µs

You can see that surprisingly, cargo build isn't the bottleneck for this case. We can use this show the impact of the uv integration :)

An annoying side effect is that tracing subscriber always shows the active spans so the logging gets more verbose.

We can also plug tracing-durations-export into maturin:

traces

The profiling profile allows using samply, perf or other profilers with maturin.

Copy link

netlify bot commented Apr 2, 2024

Deploy Preview for maturin-guide ready!

Name Link
🔨 Latest commit 4238dee
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/660be4f38a446a0008dc7ed6
😎 Deploy Preview https://deploy-preview-2019--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

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

Nice!

@messense messense enabled auto-merge April 2, 2024 11:01
@messense messense added this pull request to the merge queue Apr 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 2, 2024
@konstin
Copy link
Member Author

konstin commented Apr 2, 2024

Transient failure on freebsd:

WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'ProtocolError('Connection aborted.', ConnectionResetError(54, 'Connection reset by peer'))': /simple/pycparser/

@konstin
Copy link
Member Author

konstin commented Apr 2, 2024

Should we make log/tracing non-optional? I don't see a reason to build a maturin that doesn't support RUST_LOG. Looking through the git history, it seems i made that an optional feature way back in 2018 due to using pretty_env_logger; i'm not entirely happy with tracing_subscriber but i think it's correct to make it a mandatory dependency.

@messense
Copy link
Member

messense commented Apr 2, 2024

Sounds fine to me.

@messense messense merged commit a52843a into main Apr 2, 2024
29 checks passed
@messense messense deleted the konsti/instrument-develop branch April 2, 2024 12:34
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jun 6, 2024
https://build.opensuse.org/request/show/1178629
by user mia + anag+factory
- Update to 1.6.0
  * Add pypi name validation
    gh#PyO3/maturin#2007
  * Add JSON schema generation
    gh#PyO3/maturin#2005
  * Detect compiling from Linux gnu to Linux musl as cross compiling
    gh#PyO3/maturin#2010
  * Upgrade uniffi to 0.27.0
    gh#PyO3/maturin#2021
  * Add instrumentation support for develop
    gh#PyO3/maturin#2019
  * Make tracing-subscriber mandatory
    gh#PyO3/maturin#2022
  * Import hook upgrade
    gh#PyO3/maturin#2024
  * Add uv as develop backend command
    gh#PyO3/maturin#2015
  * Also try uv in PATH in develop --uv
    gh#PyO3/maturin#2026
  * docs: update pyo3 to match tutorial
    gh#PyO3/maturin#2029
  * Add support for AIX
    gh#PyO3/maturin#2030
  * Remove rust-cpython from project init/new template
    gh#PyO3/maturin#2034
  * Only run uv tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants