Repository navigation
feat(rocprof-compute) Add gzip compression for large profile counter CSV artifacts (Phase B) - #9711
Conversation
f26ab0b to
88a0a1f
Compare
a7e133b to
a6a4117
Compare
a6a4117 to
cbe00fc
Compare
…o users/jamessiddeley-amd/gzip-csv-compression
There was a problem hiding this comment.
Pull request overview
Adds a gzip compression boundary for large profile/analyze counter CSV artifacts in rocprofiler-compute, so profile mode writes compressed outputs while analyze mode accepts both compressed and legacy uncompressed workloads.
Changes:
- Introduces
utils.csv_compressionhelpers and updates Python profile/analyze paths to read/write.csv.gzwhere appropriate. - Adds a small C++
compressionutility library (GzipFileOutputStream) and updates the native counters writer to emit gzip-compressed CSV. - Updates tests, documentation, CI dependencies, and CMake wiring to cover/ship the new behavior.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/rocprofiler-compute/tests/test_utils.py | Updates tests to discover/read compressed results_*.csv.gz and validate empty/corrupt handling. |
| projects/rocprofiler-compute/tests/test_utils_profile_csv.py | Adds unit tests ensuring profile CSV helpers work with .csv.gz. |
| projects/rocprofiler-compute/tests/test_torch_trace_analysis.py | Adds regression test to ensure ML API trace counter copies preserve compression. |
| projects/rocprofiler-compute/tests/test_profile_general.py | Adjusts tests to locate compressed results artifacts via find_csvs. |
| projects/rocprofiler-compute/tests/test_csv_compression.py | New unit test suite for utils.csv_compression. |
| projects/rocprofiler-compute/tests/test_categories.yaml | Registers new Python and C++ compression-related tests in categories. |
| projects/rocprofiler-compute/tests/test_analysis_base.py | Adds analyze-base tests for concatenating compressed/mixed results and failing on truncation. |
| projects/rocprofiler-compute/tests/common.py | Routes CSV content scanning through open_csv_read; accepts .csv.gz for PMC artifacts. |
| projects/rocprofiler-compute/src/utils/utils_profile.py | Writes results_*.csv.gz and resolves native counter CSVs in either compressed/plain form. |
| projects/rocprofiler-compute/src/utils/utils_profile_csv.py | Opens CSVs through the compression boundary for consistent streaming behavior. |
| projects/rocprofiler-compute/src/utils/utils_analysis.py | Discovers compressed results and reports gzip truncation/corruption explicitly. |
| projects/rocprofiler-compute/src/utils/rocpd_data.py | Uses open_csv_write for CSV generation paths. |
| projects/rocprofiler-compute/src/utils/csv_compression.py | New module defining read/write/open and discovery helpers for .csv vs .csv.gz. |
| projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py | Uses compression-aware open/discovery when merging results_*.csv* into pmc_perf.csv. |
| projects/rocprofiler-compute/src/lib/utils/compression/tests/test_gzip_output_stream.h | New gtest fixture declarations for gzip stream tests. |
| projects/rocprofiler-compute/src/lib/utils/compression/tests/test_gzip_output_stream.cpp | New gtests validating gzip writer correctness and robustness. |
| projects/rocprofiler-compute/src/lib/utils/compression/tests/main.cpp | Adds gtest main for the compression test binary. |
| projects/rocprofiler-compute/src/lib/utils/compression/tests/CMakeLists.txt | Builds/links the test-compression executable. |
| projects/rocprofiler-compute/src/lib/utils/compression/gzip_output_stream.h | Adds GzipFileOutputStream API used by the counters writer. |
| projects/rocprofiler-compute/src/lib/utils/compression/gzip_output_stream.cpp | Implements a thin gzFile-based streaming writer. |
| projects/rocprofiler-compute/src/lib/utils/compression/CMakeLists.txt | Adds compression static library target and zlib linkage. |
| projects/rocprofiler-compute/src/lib/utils/CMakeLists.txt | Wires the new compression subdirectory into the utils build. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/tests/test_rocprofiler_compute_tool.cpp | Updates expected default output filename suffix to .csv.gz. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/tests/test_counters_writer.h | New gtest fixture for counters writer unit tests. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/tests/test_counters_writer.cpp | New unit tests for batching/formatting and gzip output behavior. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/tests/CMakeLists.txt | Adds counters writer tests to the test target sources. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/rocprofiler_compute_tool.cpp | Changes default native counter artifact naming to .csv.gz. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/counters_writer.h | Exposes format_counters_csv and documents gzip output expectations. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/counters_writer.cpp | Switches writer to gzip stream output and adds batched formatting. |
| projects/rocprofiler-compute/src/lib/rocprofiler_compute_tool/CMakeLists.txt | Links the tool library against the new compression library. |
| projects/rocprofiler-compute/src/lib/CMakeLists.txt | Adds zlib pkg-config target wiring for the C++ side. |
| projects/rocprofiler-compute/docs/how-to/profile/mode.rst | Documents that profile writes results_*.csv.gz and native counter .csv.gz. |
| projects/rocprofiler-compute/docs/how-to/analyze/mode.rst | Documents analyze compatibility with both .csv and .csv.gz. |
| projects/rocprofiler-compute/docs/design/profile_interface_architecture.md | Updates Phase B status/description to reflect implementation. |
| projects/rocprofiler-compute/docs/design/lld-profiler-hub-native-tool-integration.md | Updates native tool output artifact naming to .csv.gz. |
| projects/rocprofiler-compute/CMakeLists.txt | Registers new Python and C++ tests with CTest and coverage lists. |
| projects/rocprofiler-compute/CHANGELOG.md | Notes gzip compression of large counter CSV artifacts. |
| .github/workflows/rocprofiler-compute-ubuntu-jammy.yml | Adds zlib dev package to CI dependencies. |
| .github/workflows/rocprofiler-compute-tarball.yml | Adds zlib dev package to CI dependencies. |
| .github/workflows/rocprofiler-compute-sanitizers.yml | Adds zlib dev package to CI dependencies. |
| .github/workflows/rocprofiler-compute-rhel.yml | Adds zlib dev package to CI dependencies. |
| .github/workflows/rocprofiler-compute-continuous-integration.yml | Adds zlib dev package to CI dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…md/gzip-csv-compression
…stems into users/jamessiddeley-amd/gzip-csv-compression
- Write csv_compression for Python 3.8 directly instead of relying on `from __future__ import annotations`; document the rule in PYTHON_CODING_STYLE.md so it does not recur. - Qualify csv_compression calls at every call site. Bare find_csvs and open_csv_write gave a reader no hint that compression was involved. - Give CsvCountersWriter ownership of its artifact suffix, with a static_assert tying it to the gzip suffix. The filename and the format were decided in separate files with only a comment holding them together. - Narrow the corrupt-CSV handler in concat_result_csvs to the read; the pmc_perf.csv write is plain and cannot raise those errors. - Hold test-compression out of test_categories.yaml until TheRock registers it in the rocprofiler-compute post-install hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Pushed the review fixes straight to the branch instead of round-tripping comments. Python
C++ and CMake
Tests
Docs
Verified locally: unit tests and ruff clean, |
|
Both belong in the same TheRock PR. Once it merges, confirm the ref our workflows build against picks it up (the nightly tracks |
|
Please drop plain-CSV support in a follow-up PR. Profile output is an internal contract, not something we owe compatibility on, and carrying both forms is where most of the complexity here lives: |
4dff4bd
into
rocprofiler-compute-develop
…CSV artifacts (Phase B) (#9711) Profile mode writes large counter CSVs (per-pass `results_*.csv` and per-process native counter output). Gzip compression cuts the size of these intermediate csv files by **97%** with no change to peak memory, since both sides stream the compression. Analyze accepts both compressed files and uncompressed csv files, so workloads from earlier releases still work. Compression is applied at two write boundaries only: - `src/utils/csv_compression.py`: all profile/analyze CSV opens go through this module; `.csv.gz` is gzip, `.csv` is plain. - src/lib/utils/compression (C++): native tool counter output via `GzipFileOutputStream`. Both sides share a format contract documented in `docs/design/profile_interface_architecture.md`. The previous `OutputStream` factory layer was removed in favor of direct gzip writers. CI workflows add `zlib` dev packages. Truncated or corrupt gzip artifacts are reported as errors rather than partial data. <!-- JIRA ID: --> AIPROFCOMP-749 - `test_csv_compression`, `test_analysis_base`, updated counters-writer and gzip-stream C++ tests - Updated profile/analyze integration tests (`test_utils`, `test_torch_trace_analysis`) - `test-compression` and `test_counters_writer` in C++ test suite pending - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…CSV artifacts (Phase B) (#9711) Profile mode writes large counter CSVs (per-pass `results_*.csv` and per-process native counter output). Gzip compression cuts the size of these intermediate csv files by **97%** with no change to peak memory, since both sides stream the compression. Analyze accepts both compressed files and uncompressed csv files, so workloads from earlier releases still work. Compression is applied at two write boundaries only: - `src/utils/csv_compression.py`: all profile/analyze CSV opens go through this module; `.csv.gz` is gzip, `.csv` is plain. - src/lib/utils/compression (C++): native tool counter output via `GzipFileOutputStream`. Both sides share a format contract documented in `docs/design/profile_interface_architecture.md`. The previous `OutputStream` factory layer was removed in favor of direct gzip writers. CI workflows add `zlib` dev packages. Truncated or corrupt gzip artifacts are reported as errors rather than partial data. <!-- JIRA ID: --> AIPROFCOMP-749 - `test_csv_compression`, `test_analysis_base`, updated counters-writer and gzip-stream C++ tests - Updated profile/analyze integration tests (`test_utils`, `test_torch_trace_analysis`) - `test-compression` and `test_counters_writer` in C++ test suite pending - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ode CI (#7421) ## Motivation Added a `test-compression` gtest in [#9711 of rocprofiler-compute,](ROCm/rocm-systems#9711), but TheRock CI can't run it in install mode yet: - The post-hook sets install RPATH for the other gtests under `libexec/rocprofiler-compute/tests/` but not `test-compression` - The subproject pulls bundled elfutils/libdrm/sqlite3 but not zlib, so `pkg_check_modules(zlib)` can link system `libz` instead of bundled sysdeps The follow-up in rocm-systems repo is to add `test-compression` to `test_categories.yaml` after this merges. ## Technical Details - Add `test-compression` to the RPATH post-hook foreach in `profiler/post_hook_rocprofiler-compute.cmake`. - Add `${THEROCK_BUNDLED_ZLIB}` to `rocprofiler-compute` `RUNTIME_DEPS` in `profiler/CMakeLists.txt`. ## Test Plan - Build with rocprofiler-compute + tests enabled. - Confirm `test-compression` installs under `libexec/rocprofiler-compute/tests/` with correct RPATH. - Run `ctest -R test-compression`. ## Test Result - Pending CI. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
…ode CI (#7421) ## Motivation Added a `test-compression` gtest in [#9711 of rocprofiler-compute,](ROCm/rocm-systems#9711), but TheRock CI can't run it in install mode yet: - The post-hook sets install RPATH for the other gtests under `libexec/rocprofiler-compute/tests/` but not `test-compression` - The subproject pulls bundled elfutils/libdrm/sqlite3 but not zlib, so `pkg_check_modules(zlib)` can link system `libz` instead of bundled sysdeps The follow-up in rocm-systems repo is to add `test-compression` to `test_categories.yaml` after this merges. ## Technical Details - Add `test-compression` to the RPATH post-hook foreach in `profiler/post_hook_rocprofiler-compute.cmake`. - Add `${THEROCK_BUNDLED_ZLIB}` to `rocprofiler-compute` `RUNTIME_DEPS` in `profiler/CMakeLists.txt`. ## Test Plan - Build with rocprofiler-compute + tests enabled. - Confirm `test-compression` installs under `libexec/rocprofiler-compute/tests/` with correct RPATH. - Run `ctest -R test-compression`. ## Test Result - Pending CI. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
…ifacts (#10237) ## Motivation Counter CSVs are now written as `.csv.gz`, so the intermediate plain csv read paths and test fixtures are no longer needed and removing them simplifies the python compression boundary. Compressing the fixtures shrinks `tests/workloads` from 75M to 60M. ## Technical Details - Compress the marker artifacts: `out/pmc_1/<fbase>_marker_api_trace.csv` and its `ml_api_trace_*` copy come from the same rocpd databases as the counter CSV, so they are named through `compressed_name` too. Every artifact the profile CSV helpers handle is now `.csv.gz`. - Make `csv_compression` gzip-only: `open_csv_read`/`open_csv_write` become `open_gzip_csv_read`/`open_gzip_csv_write` with no suffix dispatch, and `resolve_csv` and content sniffing are gone. - Drop `find_csvs`, a one-line glob wrapper added with Phase B. `analysis_base` and `utils_analysis` glob for `GZIP_SUFFIX` directly, as they did before Phase B. - The two plain writes left, `sysinfo.csv` and the `pmc_perf.csv` analyze intermediate, use the builtin `open` at their call sites instead of routing through the compression module. - Update profile/analyze callers to use `compressed_name` directly (removes a redundant one line return helper function). - Compress the counter artifacts under `tests/workloads`: `results_*.csv`, plus the marker and `counter_collection` CSVs in the `ml_api_trace`, `torch_trace`, and `triton_trace` workloads. Drop the plain counter fixtures. - Update the Phase B and AD-2 sections of `docs/design/profile_interface_architecture.md`. - Related: Follow up to #9711 ## Issue Tracking Related: #9711 ## Test Plan - `pytest tests/unit` - `pytest tests/integration/test_analyze_workloads.py` - `pytest tests/integration/test_profile_path.py::test_path_rocpd tests/integration/test_profile_misc.py::test_analyze_rocpd tests/integration/test_profile_section.py::test_lds_section` ## Test Result All pass locally on gfx950. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ifacts (#10237) ## Motivation Counter CSVs are now written as `.csv.gz`, so the intermediate plain csv read paths and test fixtures are no longer needed and removing them simplifies the python compression boundary. Compressing the fixtures shrinks `tests/workloads` from 75M to 60M. ## Technical Details - Compress the marker artifacts: `out/pmc_1/<fbase>_marker_api_trace.csv` and its `ml_api_trace_*` copy come from the same rocpd databases as the counter CSV, so they are named through `compressed_name` too. Every artifact the profile CSV helpers handle is now `.csv.gz`. - Make `csv_compression` gzip-only: `open_csv_read`/`open_csv_write` become `open_gzip_csv_read`/`open_gzip_csv_write` with no suffix dispatch, and `resolve_csv` and content sniffing are gone. - Drop `find_csvs`, a one-line glob wrapper added with Phase B. `analysis_base` and `utils_analysis` glob for `GZIP_SUFFIX` directly, as they did before Phase B. - The two plain writes left, `sysinfo.csv` and the `pmc_perf.csv` analyze intermediate, use the builtin `open` at their call sites instead of routing through the compression module. - Update profile/analyze callers to use `compressed_name` directly (removes a redundant one line return helper function). - Compress the counter artifacts under `tests/workloads`: `results_*.csv`, plus the marker and `counter_collection` CSVs in the `ml_api_trace`, `torch_trace`, and `triton_trace` workloads. Drop the plain counter fixtures. - Update the Phase B and AD-2 sections of `docs/design/profile_interface_architecture.md`. - Related: Follow up to #9711 ## Issue Tracking Related: #9711 ## Test Plan - `pytest tests/unit` - `pytest tests/integration/test_analyze_workloads.py` - `pytest tests/integration/test_profile_path.py::test_path_rocpd tests/integration/test_profile_misc.py::test_analyze_rocpd tests/integration/test_profile_section.py::test_lds_section` ## Test Result All pass locally on gfx950. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ifacts (#10237) ## Motivation Counter CSVs are now written as `.csv.gz`, so the intermediate plain csv read paths and test fixtures are no longer needed and removing them simplifies the python compression boundary. Compressing the fixtures shrinks `tests/workloads` from 75M to 60M. ## Technical Details - Compress the marker artifacts: `out/pmc_1/<fbase>_marker_api_trace.csv` and its `ml_api_trace_*` copy come from the same rocpd databases as the counter CSV, so they are named through `compressed_name` too. Every artifact the profile CSV helpers handle is now `.csv.gz`. - Make `csv_compression` gzip-only: `open_csv_read`/`open_csv_write` become `open_gzip_csv_read`/`open_gzip_csv_write` with no suffix dispatch, and `resolve_csv` and content sniffing are gone. - Drop `find_csvs`, a one-line glob wrapper added with Phase B. `analysis_base` and `utils_analysis` glob for `GZIP_SUFFIX` directly, as they did before Phase B. - The two plain writes left, `sysinfo.csv` and the `pmc_perf.csv` analyze intermediate, use the builtin `open` at their call sites instead of routing through the compression module. - Update profile/analyze callers to use `compressed_name` directly (removes a redundant one line return helper function). - Compress the counter artifacts under `tests/workloads`: `results_*.csv`, plus the marker and `counter_collection` CSVs in the `ml_api_trace`, `torch_trace`, and `triton_trace` workloads. Drop the plain counter fixtures. - Update the Phase B and AD-2 sections of `docs/design/profile_interface_architecture.md`. - Related: Follow up to #9711 ## Issue Tracking Related: #9711 ## Test Plan - `pytest tests/unit` - `pytest tests/integration/test_analyze_workloads.py` - `pytest tests/integration/test_profile_path.py::test_path_rocpd tests/integration/test_profile_misc.py::test_analyze_rocpd tests/integration/test_profile_section.py::test_lds_section` ## Test Result All pass locally on gfx950. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Motivation
Profile mode writes large counter CSVs (per-pass
results_*.csvand per-process native counter output). Gzip compression cuts the size of these intermediate csv files by 97% with no change to peak memory, since both sides stream the compression.Analyze accepts both compressed files and uncompressed csv files, so workloads from earlier releases still work.
Technical Details
Compression is applied at two write boundaries only:
src/utils/csv_compression.py: all profile/analyze CSV opens go through this module;.csv.gzis gzip,.csvis plain.GzipFileOutputStream.Both sides share a format contract documented in
docs/design/profile_interface_architecture.md. The previousOutputStreamfactory layer was removed in favor of direct gzip writers.CI workflows add
zlibdev packages. Truncated or corrupt gzip artifacts are reported as errors rather than partial data.Issue Tracking
AIPROFCOMP-749Test Plan
test_csv_compression,test_analysis_base, updated counters-writer and gzip-stream C++ teststest_utils,test_torch_trace_analysis)test-compressionandtest_counters_writerin C++ test suiteTest Result
pending
Submission Checklist