Skip to content

Conversation

@grdumas
Copy link
Collaborator

@grdumas grdumas commented Sep 30, 2025

Description

This PR adds support for Performance Co-Pilot (PCP) metric collection during test runs. The new PCP functionality replaces the previous Pbench implementation, which is now deprecated.

Before/After Comparison

Before: wrapper provided Pbench functionality via --pbench.

After: wrapper no longer has Pbench functionality nor CLI options. Via the --use_pcp option, users can chose to create a PCP archive during the test run.

Clerical Stuff

This closes #45

Mention the JIRA ticket of the issue, this helps keep
everything together so we can find this PR easily.

Relates to JIRA: RPOPC-632

- Add --use_pcp option to enable PCP monitoring
- Integrate PCP setup, logging, and cleanup into test execution
- Add PCP iteration tracking and result collection
- PCP functionality works alongside existing pbench support
- Remove all --pbench* command line options from README
- Remove pbench execution branch from coremark_pro_run script
- Remove pbench installation note from documentation
- Simplify execution flow to use direct test execution only
@grdumas grdumas self-assigned this Sep 30, 2025
@grdumas grdumas requested a review from a team September 30, 2025 01:08
@github-actions
Copy link

This relates to RPOPC-632

kill $PMLOGGER_PID 2>/dev/null
wait $PMLOGGER_PID 2>/dev/null

stop_pcp_subset

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be shutdown_pcp, since this appears to be in the "wrapping things up" stage?

start_pcp $pcp_dir coremark-pro $pcp_cfg
start_pcp_subset

pmlogger -c $pcp_cfg -l $pcp_dir/pmlogger.log $pcp_dir/archive & PMLOGGER_PID=$!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove line.

pcp_cfg=$TOOLS_BIN/pcp/default.cfg
pcp_dir=/tmp/pcp_coremark-pro_$(date +%Y.%m.%d-%H.%M.%S)
start_pcp $pcp_dir coremark-pro $pcp_cfg
start_pcp_subset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove.

start_pcp $pcp_dir coremark-pro $pcp_cfg
start_pcp_subset

pmlogger -c $pcp_cfg -l $pcp_dir/pmlogger.log $pcp_dir/archive & PMLOGGER_PID=$!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove, it should be handled by the pcp_command.inc functions

Comment on lines 324 to 325
kill $PMLOGGER_PID 2>/dev/null
wait $PMLOGGER_PID 2>/dev/null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use shutdown_pcp per @malucius-rh's comment

pcp_cfg=$TOOLS_BIN/pcp/default.cfg
pcp_dir=/tmp/pcp_coremark-pro_$(date +%Y.%m.%d-%H.%M.%S)
start_pcp $pcp_dir coremark-pro $pcp_cfg
start_pcp_subset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this, as it is handled within the iteration loop

@kdvalin kdvalin added the group_review_lgtm Indicates approval after a group review meeting label Sep 30, 2025
dvalinrh
dvalinrh previously approved these changes Sep 30, 2025
if [ $no_overrides -eq 0 ]; then
update_build_options
make TARGET=linux64 build
exit_out "make of test failed, exiting" 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle pcp in exit_out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member

@kdvalin kdvalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@malucius-rh malucius-rh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@grdumas grdumas merged commit c65e577 into main Sep 30, 2025
3 checks passed
@grdumas grdumas deleted the Transition-from-pbench-to-Performance-Co-Pilot-(PCP) branch September 30, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group_review_lgtm Indicates approval after a group review meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace pbench with Performance Co-Pilot (PCP)

5 participants