Skip to content

Add performance logging for qdk native wheel build diagnostics#3492

Open
domorale wants to merge 3 commits into
mainfrom
feature/qdk-native-wheel-telemetry-build
Open

Add performance logging for qdk native wheel build diagnostics#3492
domorale wants to merge 3 commits into
mainfrom
feature/qdk-native-wheel-telemetry-build

Conversation

@domorale

@domorale domorale commented Jul 21, 2026

Copy link
Copy Markdown

Summary

  • Adds best-effort, non-blocking performance logging for qdk native wheel builds.
  • Moves logging helpers out of build.py into a dedicated module.
  • Logs pre-build, post-build, and failure snapshots (platform, Python version, sanitized command, system memory state).
  • Adds in-build peak indicators using OS-native tooling where available.

Scope

  • Logging is limited to the qdk native wheel build path.
  • Build outputs, flags, and packaging behavior are unchanged.
  • No new runtime dependencies were added.

Platform behavior

  • Linux: uses /usr/bin/time -v for peak indicator output.
  • macOS: uses /usr/bin/time -l for peak indicator output.
  • Windows: keeps snapshot logging; no built-in peak wrapper parity yet.

Validation

  • python3 -m py_compile performance_logging.py
  • python3 build.py --no-check-prereqs --no-check --no-test --qdk
  • python3 ./build.py

Risk

  • Low.
  • Logging is best-effort and non-blocking.
  • If logging collection fails, the build continues.

Current iteration scope

  • Rename runtime diagnostics wording to performance logging.
  • Move logging helpers out of the main build orchestration path into a dedicated module.
  • Add during-build peak indicators on Linux and macOS using OS-native tooling.
  • Keep diagnostics best-effort, non-blocking, and scoped to qdk native wheel builds.

Potential follow-up (if needed)

  • If current logs are insufficient, evaluate optional cross-platform library-based sampling (for example psutil) to complement OS-native logging.
  • If cost/benefit is favorable, add process-tree tracking during the build window to capture peak root-process RSS and peak aggregate child-process RSS.
  • If attribution remains unclear, report Top-N subprocess memory offenders near peak.
  • For reproducible failures, consider optional profiler-assisted deep dives outside the default CI path.

Comment thread build.py Outdated
Comment thread build.py
Comment thread build.py Outdated
@domorale
domorale force-pushed the feature/qdk-native-wheel-telemetry-build branch from af22ec3 to 0cc83c7 Compare July 21, 2026 21:19
@domorale domorale changed the title Add telemetry for qdk native wheel build diagnostics Add performance logging for qdk native wheel build diagnostics Jul 21, 2026
@domorale

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

Comment thread performance_logging.py Outdated
Comment thread performance_logging.py
Comment thread performance_logging.py Outdated
Comment thread performance_logging.py Outdated
Comment thread performance_logging.py Outdated
Comment thread performance_logging.py Outdated
@domorale

Copy link
Copy Markdown
Author

Optional Next Iteration Proposal (qdk native wheel diagnostics)

Current iteration objective was to improve diagnostics for the qdk native wheel step using a dependency-free, OS-native approach.
That objective is satisfied in this PR.

A potential next iteration is to evaluate optional psutil-based process metrics, mainly to improve Windows peak-memory parity and add top offender attribution.

Why consider this next iteration

  • Current gap: Windows does not have OS-native peak-wrapper parity in the current implementation.
  • psutil can provide:
    • peak largest-process RSS
    • peak process-tree RSS
    • top offenders at peak

Local experiment summary (same qdk native wheel path)

  • OS-native time max RSS: 3906428 kB (~3814.9 MiB)
  • psutil peak largest-process RSS: 3753.7 MiB (rustc)
  • psutil peak process-tree RSS: 4524.5 MiB
  • top offenders were identified with crate-level context

Interpretation:

  • largest-process peak from psutil was close to OS-native max RSS (good parity signal),
  • process-tree peak added extra attribution signal.

Cost / tradeoff

  • Adds one native dependency (psutil, ~1.1 MB installed in local test), which increases dependency/maintenance surface.

Decision

No change is proposed to current iteration scope in this PR by this comment.

If the team wants to pursue this next iteration, we can do either:

  1. A new revision on this same PR, or
  2. A separate follow-up PR.

If the team is satisfied with current status, we can keep this PR as-is and merge the dependency-free implementation.

@domorale
domorale force-pushed the feature/qdk-native-wheel-telemetry-build branch 2 times, most recently from 88de101 to 9222cea Compare July 24, 2026 03:14
@domorale

Copy link
Copy Markdown
Author

Perf logging for the qdk native wheel build is now complete and validated.

This PR:

  • adds best-effort, non-blocking perf logging for the qdk build path
  • captures structured build context and memory diagnostics
  • uses psutil process-tree sampling for peak memory tracking
  • keeps psutil scoped to the qdk build flow only
  • adds build-system test coverage for the perf logging behavior
  • hardens path sanitization and normalization across Linux, macOS, and Windows

Validation:

  • targeted perf-logging tests pass locally
  • full python build.py passes locally
  • GitHub CI passed on all platforms

domorale added 3 commits July 23, 2026 21:21
Add best-effort performance logging for the qdk native wheel build path.

Log pre-build, post-build, and failure snapshots including:
- platform
- Python version
- sanitized command
- system memory state

Keep diagnostics non-blocking and scoped to the qdk native wheel step only.

Capture peak memory indicators during build execution with OS-native tooling:
- Linux: /usr/bin/time -v
- macOS: /usr/bin/time -l
- Windows: snapshot logging with no built-in peak wrapper parity

Move performance logging helpers into a dedicated module to keep build orchestration code focused.
…ntegration

Follow up the perf logging refactor with clearer architecture and validation.

- Switch peak-memory indicators to psutil process-tree sampling.
- Keep diagnostics best-effort and non-blocking.
- Keep psutil build-time only (no runtime dependency change).
- Separate build-context reporting from system-memory snapshot phase reporting.
- Move build perf tests to tests/build and integrate them into build.py qdk test flow.
- Add psutil prereq version checks with install/update behavior.

Validation:
- python3 -m py_compile build.py prereqs.py performance_logging.py tests/build/test_performance_logging.py
- python -m pytest tests/build -q
- python build.py --qdk --test --no-check --no-check-prereqs

Architecture validation (lazy imports and scoped psutil):
- Fresh venv without psutil: wasm build succeeds (psutil not required for non-qdk targets)
- Fresh venv without psutil: qdk build auto-installs psutil (installed on-demand)
- Verified --no-check-prereqs doesn't prevent qdk psutil auto-install
- Lazy import cost: ExecutionPolicy imported only on first run() call for perf logging path,
  then cached in sys.modules; plain-path subprocess calls avoid import entirely
- Use pathlib.Path.resolve() and .relative_to() for cleaner path logic
- Use .as_posix() to ensure forward slashes on all platforms (including Windows)
- Removed os.path module usage (expanduser, isabs, basename)
- Applied DRY: extracted _detect_path_type() helper to replace 4 code repetitions
  Example: path_type = _detect_path_type(path) instead of manual conditional
- Updated test mock: os.path.expanduser → pathlib.Path.home()

Uses established QDK patterns (like _path_stem in _context.py) for cross-platform
path parsing. All 23 tests pass including Windows path sanitization.

Fixes test_sanitize_path_repo_root_and_home on Windows CI
@domorale
domorale force-pushed the feature/qdk-native-wheel-telemetry-build branch from 9222cea to bd86c16 Compare July 24, 2026 04:45
@domorale
domorale requested review from amcasey and billti July 24, 2026 05:33
@@ -0,0 +1,393 @@
import subprocess
import unittest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We use pytest for Python tests everywhere else in QDK. Would be good to use only one testing framework for all Python tests.
Is there a reason to prefer unittest over pytest for this test?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No reason at all. I agree we should use pytest.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have the update in my local branch just using pytest. On monday we'll discuss with the team about the pros/cons of adding functionality and dependencies (psutil in this PR) to the QDK build.

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.

4 participants