Repository navigation
feat(hipfile): Add getStats functions to public api - #8252
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
Pull request overview
This PR adds a public “Stats API” to hipFile (L1/L2/L3) so external tooling (e.g., RocProfiler) can query aggregated and per-GPU I/O telemetry, and wires those new entry points through the API tracing/dispatch table. It also introduces unit tests validating the new aggregation and derived-field behavior.
Changes:
- Added new public stats structs and
hipFileGetStatsL1/L2/L3API declarations tohipfile.h. - Implemented
hipFileGetStatsL1/L2/L3in the runtime and hooked them into the api-trace dispatch table. - Added unit tests covering the three new getStats functions and basic aggregation/derivation behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/hipfile/test/amd_detail/hipfile-api.cpp | Adds unit tests for the new stats APIs (L1/L2/L3). |
| projects/hipfile/src/amd_detail/hipfile.cpp | Implements hipFileGetStatsL1/L2/L3 and derives aggregate/per-GPU metrics. |
| projects/hipfile/src/amd_detail/api_trace/api-trace.cpp | Registers the new getStats functions in the dispatch table and updates ABI enforcement indices/versioning. |
| projects/hipfile/src/amd_detail/api_trace/api-trace-internal.h | Adds internal declarations for the new getStats functions. |
| projects/hipfile/src/amd_detail/api_trace/api-dispatch-interface.cpp | Exposes public C entry points that forward to the dispatch table for getStats functions. |
| projects/hipfile/include/hipfile.h | Introduces the public Stats API types and function declarations. |
| projects/hipfile/include/hipfile-api-trace.h | Bumps dispatch table step version and adds function pointer typedefs/entries for getStats APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6abbac7 to
53128ff
Compare
43be4af to
48f0522
Compare
sputhala-amd
left a comment
There was a problem hiding this comment.
Please bump the hipFile version in both CMakeLists.txt and hipfile.h so downstream consumers like rocprofiler-systems can gate on version at build time via find_package(hipfile ) and at compile time via the HIPFILE_VERSION_* macros.
Adds the public types and function declarations for the three-level stats reporting API: hipFileStatsLevel1_t, hipFileStatsLevel2_t, hipFileStatsLevel3_t, hipFilePerGpuStats_t, hipFileOpCounter_t, and the corresponding hipFileGetStatsL1/L2/L3 functions. Fields that hipFile does not track are documented as zero-filled. Calculated fields (bandwidth, average latency) are noted as derived from tracked counters. Co-Authored-By: Claude <noreply@anthropic.com>
…entations Adds hipFileGetStatsL1/L2/L3 to the profiler dispatch table: - typedefs and struct members in hipfile-api-trace.h (step version 0 -> 1) - runtime declarations in api-trace-internal.h - UpdateDispatchTable wiring and ABI enforcement assertions in api-trace.cpp - extern "C" thunks in api-dispatch-interface.cpp - stub implementations in hipfile.cpp (return hipFileInternalError) Co-Authored-By: Claude <noreply@anthropic.com>
Aggregates IO bytes, counts, latency sums, and error counts across all GPUs and both backends (fastpath + fallback). Derives average latency and bandwidth from the accumulated totals. File and buffer registration counts come from the global atomic counters. Fields with no hipFile equivalent are left zero-filled. Co-Authored-By: Claude <noreply@anthropic.com>
Delegates to hipFileGetStatsL1 for the basic counters. The 32-bucket size histograms are left zero-filled since hipFile's internal 16-bucket histograms use different bucket boundaries that cannot be losslessly mapped to the cufile format. Co-Authored-By: Claude <noreply@anthropic.com>
Delegates to hipFileGetStatsL2 for the aggregate L2 stats, then iterates all 16 GPU slots to populate per-GPU entries for active GPUs. Per-GPU mapping: - fastpath backend -> n_nvfs_reads / n_nvfs_writes - fallback backend -> n_posix_reads / n_posix_writes - bandwidth derived from bytes and duration per GPU - buffer registrations (global counter) reported on each active GPU as the best available approximation num_gpus reflects the count of GPUs with recorded activity. Co-Authored-By: Claude <noreply@anthropic.com>
48f0522 to
5a8edcf
Compare
## Motivation RocProfiler needs a public api for consuming telemetry from HipFile. ## Technical Details The public stats structs don't exactly match the internal data collected by the library, so simple (although sizeable) transformations are performed when populating the out params in hipFileGetStatsLX(). The new functions are added to the API tracing wrapper table, this will probably require coordinating with the profiler team. ## JIRA ID JIRA ID : AIHIPFILE-240 ## Test Plan Unit tests added for all three getStats functions. ## Test Result Tests passed. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. [rocm-systems] ROCm/rocm-systems#8252 (commit a3c059c)
…9260) ## Motivation The stats API changes are causing build failures in the profiler code. We are reverting this temporarily and will set up a PR once we've ensured that our API changes will no longer break the profiler. ## Technical Details This reverts commit a3c059c. ## Issue Tracking JIRA ID: AIHIPFILE-240 ## Test Plan CI passes ## Test Result Success ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
## Motivation RocProfiler needs a public api for consuming telemetry from HipFile. ## Technical Details The public stats structs don't exactly match the internal data collected by the library, so simple (although sizeable) transformations are performed when populating the out params in hipFileGetStatsLX(). The new functions are added to the API tracing wrapper table, this will probably require coordinating with the profiler team. ## JIRA ID JIRA ID : AIHIPFILE-240 ## Test Plan Unit tests added for all three getStats functions. ## Test Result Tests passed. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
…9260) ## Motivation The stats API changes are causing build failures in the profiler code. We are reverting this temporarily and will set up a PR once we've ensured that our API changes will no longer break the profiler. ## Technical Details This reverts commit a3c059c. ## Issue Tracking JIRA ID: AIHIPFILE-240 ## Test Plan CI passes ## Test Result Success ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
#9584) ## Motivation The hipFile stats API was temporarily reverted when TheRock CI failed due to issues with the profiler SDK, which includes a vendored version of hipfile.h that was not updated. ## Technical Details This reverts commit 3aabde9. This restores commit a3c059c, which added the hipFile Stats API. This PR includes the necessary ABI checks that will be necessary for TheRock CI to pass. ## Issue Tracking JIRA ID : AIHIPFILE-240 ## Test Plan CI passes ## Test Result Success ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
## Motivation <!-- Explain the purpose of this PR and the goals it aims to achieve. --> ROCm Systems Profiler should be able to provide storage telemetry - letting users correlate file/buffer registrations, read/write throughput, fast-path vs. fallback usage, and I/O errors with GPU and application activity in a single Perfetto trace or RocPD database. This is increasingly important for GPU-direct storage workloads where the storage path is a common performance bottleneck that was previously invisible in the profiler. Depends on: - [x] #8252 - [x] #9584 - [x] #10548 - [x] ROCm/TheRock#7982 Todo: - [x] Ensure version gating for hipFile public API on 0.5.0? ## Technical Details <!-- Explain the changes along with any relevant GitHub links. --> - **hipFile backend wrapper** (`source/lib/backends/hipfile/`) - Thin policy seam around `hipFileGetStatsL3`; isolates the profiler from hipFile's C API and enables mock-based unit tests. - **PMC collector** (`library/pmc/collectors/hipfile/`) - Snapshots hipFile's per-GPU stats on the process-sampling thread and stores the whole snapshot as a single typed `hipfile_pmc_sample` per GPU per poll. - **Trace-cache sample type (`source/lib/core/trace_cache/`) - Adding the typed sample follows the existing `gpu_pmc_sample` / `ainic_pmc_sample` pattern and uses the shared plumbing. - **Metric groups and defaults** - `ROCPROFSYS_HIPFILE_METRICS` selects read/write pairs by group (`fastpath`, `fallback`, `bandwidth`, `bytes`, `ops`, `unaligned`, `errors`, or `all`/`none`). - Defaults to `fastpath, fallback, bandwidth, bytes, errors` so traces focus on path usage and transfer rate without collecting every counter. - `ROCPROFSYS_HIPFILE_METRICS=none` skips device enumeration.` - **Build-time and runtime gating** - `ROCPROFSYS_BUILD_HIPFILE` is tri-state `AUTO` / `ON` / `OFF` (default `AUTO`). - `cmake/Packages.cmake` sets a derived INTERNAL `ROCPROFSYS_HIPFILE_SUPPORT` (AINIC-style) and never shadows the user cache: AUTO + missing/too-old package → disable, STATUS ON + missing/too-old package → FATAL_ERROR (version found, version required, hipfile_DIR / CMAKE_PREFIX_PATH) OFF → never search - `ROCPROFSYS_USE_HIPFILE` (env/config, default OFF) enables collection at run time; zero overhead when disabled or when the target does not use hipFile. - The backend compiles against `hipfile.h` but does not link `libhipfile`. `wrapper.hpp` resolves the two entry points it uses (`hipFileGetVersion`, `hipFileGetStatsL3`) with `dlopen`/`dlsym` at first sample. - Consequently `libhipfile.so.0` is not required for the profiler to start. Absent library, missing stats API (`hipFile` 0.4 reuses SONAME .so.0), or a too-old reported version all degrade to a warning and no hipFile tracks. - **Runtime guards** - Does not set `HIPFILE_STATS_LEVEL` (hipFile already defaults to 1). `USE_HIPFILE=ON` with `HIPFILE_STATS_LEVEL=0`, or with `USE_PROCESS_SAMPLING=OFF`, logs a warning and produces no hipFile tracks. - **Version checks** - Requires hipFile 0.5.0+ at build time (for `hipFileGetStatsL3`) and re-checks the loaded `libhipfile` at run time; logs a warning and skips collection if the runtime is too old. - **Example workload and validation** - `examples//hipfile/hipfile-io.cpp` runs a short read/write loop for telemetry validation. - `hipfile-io` is skipped when `ROCPROFSYS_HIPFILE_SUPPORT` is off - `tests/pytest/test_hipfile_telemetry.py` plus RocPD/Perfetto validation rules under `tests/rocpd-validation-rules/hipfile-telemetry/`. - **Documentation and changelog** - How-to: `docs/how-to/hipfile-telemetry.rst` (build/run settings, metric interpretation, linking contract, troubleshooting). - CHANGELOG entry under the unreleased 1.9.0 section. - **Follow-up for future** - Expose remaining hipFile telemetry that is already collected but not shown to the user. - Emit process-scoped file/buffer registration counters on a process track (as the CPU collector does), if useful. - Add `rocprof-sys-avail` reporting of which hipFile metrics can be collected. - Add a `--hipfile[=metrics]` domain flag so storage telemetry composes with existing presets (do not enable hipFile inside `trace-gpu` / `--gpu`; no dedicated storage preset until that flag exists). - When hipFile fills per-slot UUID (AIHIPFILE-272), join those to SDK agents (same as AMD SMI) so permutations/HIP_VISIBLE_DEVICES UUID lists map correctly without parsing the env var. ## Issue Tracking <!-- Include a GitHub Issue and/or JIRA ID. Do not post any full JIRA links here. --> <!-- GitHub issue: #1234 --> <!-- JIRA ID: EXAMPLECOMPONENT-1234 --> JIRA ID: AIPROFSYST-710 ## Test Plan <!-- Explain any relevant testing done to verify this PR. --> - [ ] Unit tests: `hipfile-backend-tests`, `pmc-hipfile-collector-tests` - [ ] Integration: `ctest -R hipfile` / `pytest tests/pytest/test_hipfile_telemetry.py` (requires GPU + hipFile build) - [ ] Manual: profile a hipFile workload with `ROCPROFSYS_USE_HIPFILE=ON` and confirm `hipFile GPU<N> <metric>` tracks in Perfetto/RocPD ## Test Result <!-- Briefly summarize test outcomes. --> <img width="4062" height="952" alt="image" src="https://github.com/user-attachments/assets/db32f269-ad9e-4649-a958-807e72b604e7" /> <img width="4624" height="1164" alt="image" src="https://github.com/user-attachments/assets/d137757a-c3e9-4e4d-8ac8-b8201c57f6ea" /> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: David Galiffi <David.Galiffi@amd.com> Co-authored-by: David Galiffi <David.Galiffi@amd.com>
Motivation
RocProfiler needs a public api for consuming telemetry from HipFile.
Technical Details
The public stats structs don't exactly match the internal data collected by the library, so simple (although sizeable) transformations are performed when populating the out params in hipFileGetStatsLX().
The new functions are added to the API tracing wrapper table, this will probably require coordinating with the profiler team.
JIRA ID
JIRA ID : AIHIPFILE-240
Test Plan
Unit tests added for all three getStats functions.
Test Result
Tests passed.
Submission Checklist