Add Nsight trace analysis to profiling tools - #2878
Conversation
Export NVTX host and GPU projection reports through an argument-safe subprocess boundary. Parse sanitized Nsight 2025.1 fixtures while preserving raw and normalized range names.
Summarize host-inclusive, host-exclusive, and GPU-projected NVTX timings without conflating their semantics. Link the stable analysis.json artifact to its manifest, trace, reports, and Nsight version.
Explain the analysis command, generated artifacts, and host-versus-GPU timing semantics. Add an agent workflow for interpreting results without overstating regressions.
Prefix the copy-and-paste profiling command with mkdir -p so Nsight writes the trace beside its manifest instead of falling back to a temporary path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fba24e1d51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use per-instance GPU projection traces and join host and GPU ranges by process, thread, and range ID. Validate the exact manifest iteration index set and cover target range name collisions.
| command.extend(["--seed", str(config.seed)]) | ||
|
|
||
| nsys_command = " \\\n ".join(_quote_nsys_command_part(part) for part in command) | ||
| mkdir_command = " ".join( |
There was a problem hiding this comment.
Let's just use pathlib here to create a nested directory if it doesn't exist
There was a problem hiding this comment.
Yep, this is cleaner. I moved the directory creation into build_nsys_command() using run_dir.mkdir(parents=True, exist_ok=True), so the generated command no longer needs a mkdir -p prefix.
I also changed the smoke test to check that the nested directory exists before checking the generated nsys command.
| """Analyze one profiling run and write ``analysis.json``.""" | ||
| manifest_path = run_dir / "manifest.yaml" | ||
| manifest = _load_manifest(manifest_path) | ||
| resolved_trace_path = trace_path or run_dir / "trace.nsys-rep" |
There was a problem hiding this comment.
Do we need the independent trace_path with run_dir / "trace.nsys-rep"? We can prove provenance as we have run_dir / "manifest.yaml". Allowing for independent trace_path gives the user the opportunity to make a mistake.
There was a problem hiding this comment.
I removed the separate --trace option. The run directory is now the unit of analysis: the analyzer reads both manifest.yaml and trace.nsys-rep from it.
To make sure this wasn’t only a CLI change, I updated the analyze_run() test to capture the arguments passed to run_nsys_stats() and confirm that it always receives <run-dir>/trace.nsys-rep.
| raise NsysStatsError(f"Could not read Nsight report {path}: {error}") from error | ||
|
|
||
| if not rows: | ||
| raise NsysStatsError(f"Nsight report contains no rows: {path}") |
There was a problem hiding this comment.
This could fail for parse_nvtx_gpu_projection_trace when none of the nvtx ranges have attributable GPU work, while still the other report may be valid - we just measured CPU work.
There was a problem hiding this comment.
This turned out to be a little more subtle than I initially expected. My first change allowed a header-only GPU report, but a real CPU-only capture showed that Nsight 2025.1 produces a zero-byte GPU CSV in this case.
I updated the GPU parser to handle both forms and reran the same trace. The analyzer kept the host measurements, returned null for the per-iteration GPU projections, and added warnings rather than reporting zero GPU time.
This exception is limited to empty GPU reports. A non-empty report that is missing the required columns still fails parsing.
| *, | ||
| trace_path: Path, | ||
| output_dir: Path, | ||
| executable: str = "nsys", |
There was a problem hiding this comment.
Minor. Wondering whether we should add a version check here, perhaps with only a warning. The column names in the stat files are hardcoded, so we might fail on future major version bumps, although of course those might also change commands themselves.
There was a problem hiding this comment.
I added this as a warning rather than a version gate. The current report schema is marked as validated against Nsight 2025.1. Other versions produce a warning, but parsing can continue if the required columns are still present.
I checked both cases on an NVIDIA L4 instance provisioned through JarvisLabs.ai. The Nsight 2025.1.3 integration run completed without a version warning, while the older 2024.3.2 binary installed on the same instance produced the compatibility warning. Missing report columns remain an error regardless of the version.
| class GpuProjectedRange: | ||
| """GPU work projected from one NVTX range instance.""" | ||
|
|
||
| name: str |
There was a problem hiding this comment.
I would explicitly use here a key scoped name. So instead resize we use preprocessing.resize and postprocessing.resize omitting the iteration. So we don't aggregate a resize that is being used under two different operations.
There was a problem hiding this comment.
I changed the grouping identity to use the NVTX ancestry below the harness iteration. Internally, the scope remains a tuple. The output renders it as a dotted name such as preprocessing.resize. This lets the same operation aggregate across iterations without merging it with postprocessing.resize.
The GPU summaries also use the host report as their main source of ancestry because the GPU-projection report may omit a parent with no directly attributable GPU work.
To check this against a real trace, I captured a temporary CUDA target containing both preprocessing > resize and postprocessing > resize across two iterations. The analysis reported two instances of each scoped name separately, with the corresponding GPU operations and no warnings.
Create run directories before printing Nsight commands, bind analysis to the trace stored with its manifest, accept empty GPU projection exports, and capture the Nsight version before report generation.
Scope repeated range names by NVTX ancestry below harness iterations, retain host analysis when GPU projections are absent, and warn when report parsing runs against an unvalidated Nsight version.
Summary
This adds a scriptable analysis step for traces produced by the profiling harness.
nsys statsagainst an existing.nsys-reptrace and export thenvtx_pushpop_traceandnvtx_gpu_proj_tracereports as CSV.analysis.jsonfile linked to the run manifest.iteration 0without being misclassified.Why
The profiling harness records structured NVTX ranges in an Nsight Systems trace, but reviewing those traces requires the desktop UI or manual report commands. This change provides a repeatable command that produces machine-readable summaries while preserving links to the source trace, manifest, Git commit, and Nsight version.
Run the analysis with:
Testing
PYTHONPATH=. .venv/bin/python -m pytest tests/development/profiling -q(26 passed)flake8 --select=E9,F63,F7,F82on all changed Python modulestrace.nsys-rep, both CSV reports, andanalysis.jsoniteration 00and1Scope
This change does not add performance regression thresholds. It exports measurements and provenance so threshold or comparison policies can be added separately.