Create public api for stats clients by zbyrne · Pull Request #236 · ROCm/hipFile

zbyrne · 2026-03-24T19:45:08Z

Creates a C API that wraps StatsClient and updates ais-stats to use the new API

Copilot

Pull request overview

This PR introduces a small public C API wrapper around the existing hipFile::StatsClient (to enable non-C++ consumers) and migrates the ais-stats tool to use that wrapper.

Changes:

Added a new exported C header/API (include/hipfile-stats.h) and its implementation wrapper (src/amd_detail/hipfile-stats.cpp) around hipFile::StatsClient.
Updated tools/ais-stats to use the new C API instead of directly instantiating StatsClient.
Made StatsClient::pollProcess() and StatsClient::generateReport() const to support use via a const context pointer.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tools/ais-stats/ais-stats.cpp	Switches the CLI tool to the new C API context/connect/poll/report flow.
src/amd_detail/stats.h	Marks `StatsClient` polling/report methods as `const` for wrapper usage.
src/amd_detail/stats.cpp	Implements the `const`-qualified `StatsClient` methods.
src/amd_detail/hipfile-stats.cpp	New C API wrapper implementation around `StatsClient`.
src/amd_detail/CMakeLists.txt	Adds the new wrapper source file to the AMD detail library build.
include/hipfile-stats.h	New public C header exposing the stats API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/amd_detail/hipfile-stats.cpp

include/hipfile-stats.h

src/amd_detail/hipfile-stats.cpp

Copilot · 2026-03-24T20:19:18Z

src/amd_detail/hipfile-stats.cpp

+    std::string report = stream.str();
+    if (write(fd, report.c_str(), report.size()) == -1) {
+        return HipFileStatsReportGenerationFailed;
+    }


generateReport() writes the report with a single write() call and treats any non--1 return as success. write() can legally return a short byte count (and can be interrupted with EINTR), which would truncate the report while still returning HipFileStatsSuccess. Consider looping until all bytes are written (handling EINTR) and treating short writes as failure.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/amd_detail/include_internal/hipfile-stats.h

src/amd_detail/hipfile-stats.cpp

Copilot · 2026-03-25T17:28:30Z

src/amd_detail/hipfile-stats.cpp

+    size_t      written{0};
+    while (written < total_size) {
+        ssize_t n{write(fd, data + written, total_size - written)};
+        if (n < 0) {
+            return hipFileStatsReportGenerationFailed;
+        }
+        written += static_cast<size_t>(n);
+    }


The write loop treats any write() error as fatal, but does not handle EINTR (should retry) and can spin forever if write() ever returns 0. Update the loop to retry on EINTR and treat n == 0 as an error to avoid an infinite loop.

tools/ais-stats/ais-stats.cpp

test/amd_detail/hipfile-stats.cpp

src/amd_detail/include_internal/hipfile-stats.h

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T17:50:28Z

src/amd_detail/hipfile-stats.cpp

+        return hipFileStatsInvalidArgument;
+    }
+    const IStatsClient *client{reinterpret_cast<const IStatsClient *>(context)};
+    if (!client->pollProcess(timeoutMs)) {


hipFileStatsPollTargetProcess treats a false return from IStatsClient::pollProcess as hipFileStatsTargetProcessNotAccessible. StatsClient::pollProcess returns false on a normal timeout (poll() == 0) as well as on poll() errors, so callers using a finite timeout will get an error code for an expected timeout condition. Consider adding a distinct timeout error code (and optionally distinguishing poll() errors) or updating the API contract so the return code reflects timeout vs. failure.

Suggested change

if (!client->pollProcess(timeoutMs)) {

if (!client->pollProcess(timeoutMs)) {

// For finite timeouts, a false return can indicate a normal timeout.

// Distinguish this from an actual accessibility failure.

if (timeoutMs >= 0) {

return hipFileStatsTimeout;

}

src/amd_detail/hipfile-stats.cpp

zbyrne force-pushed the zbyrne/export-stats-client branch 2 times, most recently from 32d6697 to 79e93b7 Compare March 24, 2026 19:56

zbyrne marked this pull request as ready for review March 24, 2026 20:12

zbyrne requested a review from kurtmcmillan as a code owner March 24, 2026 20:12

Copilot AI review requested due to automatic review settings March 24, 2026 20:12

zbyrne requested review from derobins, gaoikawa, jordan-turbofish and riley-dixon as code owners March 24, 2026 20:12

Copilot started reviewing on behalf of zbyrne March 24, 2026 20:14 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

zbyrne added 2 commits March 25, 2026 11:38

Add const to StatsClient methods.

bf38a34

Extract abstract base class from StatsClient to unit test stats API.

6b441a6

zbyrne force-pushed the zbyrne/export-stats-client branch 2 times, most recently from c5b6eb2 to c6c72e2 Compare March 25, 2026 17:22

Copilot AI review requested due to automatic review settings March 25, 2026 17:22

Copilot AI reviewed Mar 25, 2026

View reviewed changes

zbyrne force-pushed the zbyrne/export-stats-client branch from c6c72e2 to ccbb060 Compare March 25, 2026 17:38

Copilot AI review requested due to automatic review settings March 25, 2026 17:44

zbyrne force-pushed the zbyrne/export-stats-client branch from ccbb060 to 00c742b Compare March 25, 2026 17:44

Copilot started reviewing on behalf of zbyrne March 25, 2026 17:46 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

zbyrne added 2 commits March 25, 2026 15:20

Create exported C api for using StatsClient.

b989df8

Update ais-stats to use new API.

400fe1f

zbyrne force-pushed the zbyrne/export-stats-client branch from 00c742b to 400fe1f Compare March 25, 2026 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create public api for stats clients#236

Create public api for stats clients#236
zbyrne wants to merge 4 commits intodevelopfrom
zbyrne/export-stats-client

zbyrne commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    if (!client->pollProcess(timeoutMs)) {
+    if (!client->pollProcess(timeoutMs)) {
+        // For finite timeouts, a false return can indicate a normal timeout.
+        // Distinguish this from an actual accessibility failure.
+        if (timeoutMs >= 0) {
+            return hipFileStatsTimeout;
+        }

Conversation

zbyrne commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants