[QingleiCao]

Add parsec_device_show_capabilities_start() and parsec_device_show_capabilities_end() to report device statistics for an explicit execution interval.

Preserve whole-execution reporting when the APIs are unused. Print an unfinished interval during finalization, allow a second start to restart the interval, warn on an unmatched end, and suppress duplicate reports after a completed interval.

Snapshot cumulative device counters at the beginning of an interval and report their deltas at the end. Rebase the snapshot when statistics are reset to prevent counter underflow, and correctly handle MCA parameter index zero.

Add a PTG regression using block-cyclic matrix descriptors and real GPU data movement. Cover explicit, default, unfinished, restarted, reset, unmatched-end, and disabled reporting for both CUDA and HIP, including exact validation of the reported task and transfer statistics.

[Copilot]

🔵 Needs a closer look

Resolve statistics-reset consistency during active reporting windows.

Pull request overview

Adds scoped device-statistics reporting APIs with CUDA/HIP test coverage.

Changes:

• Adds parsec_device_show_capabilities_start/end.
• Implements scoped statistics tracking and finalization.
• Adds GPU workload and CUDA/HIP lifecycle tests.

Review note: resetting statistics during an active window can cause invalid interval reporting; this needs to be handled or explicitly restricted.

File summaries

File	Summary
tests/runtime/cuda/Testings.cmake	Registers CUDA/HIP tests and output checks.
tests/runtime/cuda/device_show_capabilities.jdf	Adds the validation workload and scenarios.
tests/runtime/cuda/CMakeLists.txt	Builds the new test executable.
parsec/runtime.h	Declares and documents the new APIs.
parsec/mca/device/device.c	Implements scoped statistics tracking and reporting.

Review details

Suppressed comments (1)

parsec/mca/device/device.c:619

• This snapshot is not kept consistent with the existing public parsec_mca_device_reset_statistics() and parsec_mca_device_dump_and_reset_statistics() APIs. If either reset is called between start and end, the counters can become smaller than parsec_device_statistics_start; the subtraction in parsec_devices_print_statistics() then triggers its assertion in debug builds or unsigned-underflows in release builds, producing an invalid interval report. Rebase the active snapshot when statistics are reset, or explicitly reject/document resets while a window is active.

    parsec_devices_save_statistics(&parsec_device_statistics_start);
    parsec_device_statistics_window_used = 1;

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

No unresolved review issues were identified.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[devreal]

I like the idea of providing an API for that. However, resetting the global counter is not intuitive. I would suggest adding an argument into which the current counters are read during start and then taking the diff to that snapshot during end, and freeing the memory that the counters occupy.

[bosilca]

1. The public names collide with a different MCA feature. device_show_capabilities already means “print hardware info at init.” These APIs control device_show_statistics.
2. if you really want such an API it should be called show_statistics or something.
3. What does it give you that the already existing API, parsec_device_show_capabilities_end, does not already provides ?
4. The current API allows multiple statistics interval ongoing. This API allows only for one.
5. Rename the capability and the test.

[QingleiCao]

1. The public names collide with a different MCA feature. device_show_capabilities already means “print hardware info at init.” These APIs control device_show_statistics.
2. if you really want such an API it should be called show_statistics or something.
3. What does it give you that the already existing API, parsec_device_show_capabilities_end, does not already provides ?
4. The current API allows multiple statistics interval ongoing. This API allows only for one.
5. Rename the capability and the test.

Thanks for the feedback.

1. Renamed.
2. parsec_device_show_statistics_start() and parsec_device_show_statistics_end().
3. The existing requires to manage its lifecycle.
4. Each start() now returns an independent handle, allowing multiple statistics intervals to overlap.
5. Renamed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[bosilca]

What does this buy over the API that is already there? parsec_devices_save_statistics(), parsec_devices_print_statistics() and parsec_devices_free_statistics() have been public in device.h for years, and together they already are a scoped interval:

uint64_t *s = NULL;
parsec_devices_save_statistics(&s);
/* ... region of interest ... */
parsec_devices_print_statistics(ctx, s);
parsec_devices_free_statistics(&s);

Overlapping intervals come for free, because each one is a local variable rather than an entry in a global list. What the 94 new lines in device.c add on top of that is only: gating on the device_show_statistics MCA parameter, suppressing the whole-execution report at parsec_fini(), rebasing baselines when statistics are reset, and printing intervals that were never ended. That is a new public opaque type in runtime.h, a malloc per interval and a process-global list, in exchange for four lines the application can already write. If the thing you actually want is "do not also print the global report at the end," that is a value of the existing MCA parameter or an argument to print_statistics, not a new API.

Note also that nothing in the tree uses the existing triplet, so if it is insufficient, the fix is more likely to be in print_statistics than beside it.

There is also a real problem with how the new tests check themselves, see the inline comment.

[bosilca]

Eighteen of the twenty new ctest registrations set PASS_REGULAR_EXPRESSION, and ctest ignores the process exit code whenever that property matches:

add_test(NAME t COMMAND sh -c "echo HELLO; exit 1")
set_tests_properties(t PROPERTIES PASS_REGULAR_EXPRESSION "HELLO")
-> 1/1 Test #1: t ... Passed

So matrix_check() never fails a test. The GPU can write the wrong bytes into every tile, the test can return EXIT_FAILURE, and ctest still reports green as long as the statistics line is printed. Only the two disabled registrations, which set no pass regex, honour the exit code.

The same property is why six of the registrations are redundant: default/default_matrix, overlap/overlap_inner and end_without_start/end_without_start_default run identical commands and differ only in which regex they check, because multiple pass regexes are OR-ed rather than AND-ed. That is six extra full GPU runs for no additional coverage.

Both of these go away if the checking moves into the test binary. It already knows how many tasks and bytes to expect, so it can take its own snapshot around each phase with save_statistics, compare against the expected counters, and return a status -- which ctest would then actually respect.