Skip to content

Continuous Benchmarking #1033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Continuous Benchmarking #1033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3eff4cb
bench_diff improve
Nov 10, 2025
a2eaec7
implement CB on bench.yml
Nov 10, 2025
8499ec5
lint fixes
Nov 10, 2025
83fd1b2
All Necessary Changes Implemented
Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/bench.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ jobs:

- name: Generate & Post Comment
run: |
git clone "${{ secrets.DOC_PUSH_URL }}" ../www
cp ../www/documentation/data.js ./pr/data.js
(cd pr && . ./mfc.sh load -c ${{ matrix.flag }} -m g)
(cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml)
(cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml -f data.js -n "${{ matrix.name }} (${{ matrix.device }}/${{ matrix.interface }})")

- name: Print Logs
if: always()
Expand Down
168 changes: 168 additions & 0 deletions .github/workflows/cont-bench.yml
View file
Open in desktop
Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 10, 2025

Choose a reason for hiding this comment

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

High-level Suggestion

The current benchmark data update process in cont-bench.yml is complex and fragile, using both a GitHub Action and manual sed commands. This should be consolidated into a single, robust mechanism, ideally by extending the Python toolchain to handle data transformations before pushing. [High-level, importance: 9]

Solution Walkthrough:

Before:

# in .github/workflows/cont-bench.yml
# ...
  - name: Post Benchmark Results
    uses: benchmark-action/github-action-benchmark@v1
    with:
      # ... (configuration)
      auto-push: true
      gh-repository: 'github.com/MFlowCode/MFlowCode.github.io'
      benchmark-data-dir-path: 'documentation'

  - name: Polishing Results
    run:  |
      # ...
      git clone "${{ secrets.DOC_PUSH_URL }}" ../www
      sed -i 's/"unit": "s\/iter"/"unit": "s"/g; s/cpu: /runtime: /g; ...' ../www/documentation/data.js
      sed -i "s/${GITHUB_SHA::7}/$TAG/g" ../www/documentation/data.js
      git -C ../www add -A
      git -C ../www commit -m "..." || true
      git -C ../www push

After:

# in .github/workflows/cont-bench.yml
# ...
  - name: Post and Polish Benchmark Results
    uses: benchmark-action/github-action-benchmark@v1
    with:
      # ... (configuration)
      auto-push: true
      gh-repository: 'github.com/MFlowCode/MFlowCode.github.io'
      benchmark-data-dir-path: 'documentation'
      # Use a script to post-process the benchmark output before it's committed
      # This script would be part of the MFC toolchain
      post-process-script: './mfc.sh bench_polish_results'

# in toolchain/mfc/bench.py
def polish_results():
    # Load data.js
    # Perform transformations in Python (robustly)
    # e.g., replace units, keys, and tags
    # Save modified data.js
    # The action would then commit and push this polished file.

Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
name: 'Continuous Benchmarking'

on:
pull_request_review:
types: [submitted]
workflow_dispatch:
inputs:
tag:
description: 'tag to CB'
required: true

permissions:
contents: write
deployments: write
pages: write
id-token: write

jobs:
file-changes:
name: Detect File Changes
runs-on: 'ubuntu-latest'
outputs:
checkall: ${{ steps.changes.outputs.checkall }}
steps:
- name: Clone
uses: actions/checkout@v4

- name: Detect Changes
uses: dorny/paths-filter@v3
id: changes
with:
filters: ".github/file-filter.yml"

self:
name: "${{ matrix.name }} (${{ matrix.device }})"
needs: file-changes
strategy:
fail-fast: false
matrix:
include:
- cluster: phoenix
name: Georgia Tech | Phoenix (NVHPC)
group: phoenix
labels: gt
flag: p
device: cpu
interface: none
build_script: ""
- cluster: phoenix
name: Georgia Tech | Phoenix (NVHPC)
group: phoenix
labels: gt
flag: p
device: gpu
interface: acc
build_script: ""
- cluster: phoenix
name: Georgia Tech | Phoenix (NVHPC)
group: phoenix
labels: gt
flag: p
device: gpu
interface: omp
build_script: ""
- cluster: frontier
name: Oak Ridge | Frontier (CCE)
group: phoenix
labels: frontier
flag: f
device: gpu
interface: acc
build_script: "bash .github/workflows/frontier/build.sh gpu acc bench"
runs-on:
group: ${{ matrix.group }}
labels: ${{ matrix.labels }}
timeout-minutes: 1400
env:
ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
steps:
- name: Clone - PR
uses: actions/checkout@v4
with:
path: pr

- name: Clone - Master
uses: actions/checkout@v4
with:
repository: MFlowCode/MFC
ref: master
path: master

- name: Setup & Build
if: matrix.build_script != ''
run: |
(cd pr && ${{ matrix.build_script }}) &
(cd master && ${{ matrix.build_script }}) &
wait %1 && wait %2

- name: Bench (Master v. PR)
run: |
(cd pr && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
(cd master && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
wait %1 && wait %2
Comment on lines +98 to +104
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

The 'wait %1 && wait %2' syntax is for Windows batch/PowerShell, not bash. In bash, you should use 'wait' without arguments to wait for all background jobs, or use process IDs. This will cause the workflow to fail when executed in a bash shell.

Suggested change
wait %1 && wait %2
- name: Bench (Master v. PR)
run: |
(cd pr && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
(cd master && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
wait %1 && wait %2
wait
- name: Bench (Master v. PR)
run: |
(cd pr && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
(cd master && bash .github/workflows/${{ matrix.cluster }}/submit-bench.sh .github/workflows/${{ matrix.cluster }}/bench.sh ${{ matrix.device }} ${{ matrix.interface }}) &
wait

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

Same issue as above - 'wait %1 && wait %2' is Windows syntax, not bash. Use 'wait' alone or capture PIDs and wait for them explicitly.

Suggested change
wait %1 && wait %2
wait

Copilot uses AI. Check for mistakes.

- name: Generate & Post Comment
run: |
git clone "${{ secrets.DOC_PUSH_URL }}" ../www
cp ../www/documentation/data.js ./pr/data.js
(cd pr && . ./mfc.sh load -c ${{ matrix.flag }} -m g)
(cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml -f data.js -n "${{ matrix.name }} (${{ matrix.device }}/${{ matrix.interface }})")

- name: Prep Benchmark Results
run: |
# should fill in the template in pr/cont-bench-template.json

Comment on lines +113 to +116
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

This step contains only a TODO comment and performs no actual work. Either implement the functionality or remove the step if it's not needed yet. Leaving empty placeholder steps in production workflows can be confusing.

Suggested change
- name: Prep Benchmark Results
run: |
# should fill in the template in pr/cont-bench-template.json

Copilot uses AI. Check for mistakes.
- name: Post Benchmark Results
uses: benchmark-action/github-action-benchmark@v1
with:
name: "${{ matrix.name }} (${{ matrix.device }}/${{ matrix.interface }})"
tool: 'googlecpp'
output-file-path: pr/report.json
github-token: ${{ secrets.TOKEN }}
auto-push: true
alert-threshold: '200%'
comment-on-alert: true
fail-on-alert: true
gh-repository: 'github.com/MFlowCode/MFlowCode.github.io'
gh-pages-branch: 'main'
benchmark-data-dir-path: 'documentation'

- name: Polishing Results
run: |
TAG="${{ github.event.inputs.tag || github.ref_name }}"
echo "tag=$TAG" >> $GITHUB_OUTPUT
echo "TAG=$TAG" >> $GITHUB_ENV

set +e
git ls-remote "${{ secrets.DOC_PUSH_URL }}" -q
if [ "$?" -ne "0" ]; then exit 0; fi
set -e
git config --global user.name 'MFC Action'
git config --global user.email '<>'
git clone "${{ secrets.DOC_PUSH_URL }}" ../www
sed -i 's/"unit": "s\/iter"/"unit": "s"/g; s/cpu: /runtime: /g; s/iterations: /time steps: /g' ../www/documentation/data.js
sed -i "s/${GITHUB_SHA::7}/$TAG/g" ../www/documentation/data.js

git -C ../www add -A
git -C ../www commit -m "Docs @ ${GITHUB_SHA::7}" || true
git -C ../www push
Comment on lines +132 to +150
Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 10, 2025

Choose a reason for hiding this comment

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

Suggestion: To fix a race condition in the Polishing Results step, implement a retry loop for the git push command. Before pushing, pull the latest changes with rebase to handle concurrent updates from other jobs. [possible issue, importance: 8]

Suggested change
- name: Polishing Results
run: |
TAG="${{ github.event.inputs.tag || github.ref_name }}"
echo "tag=$TAG" >> $GITHUB_OUTPUT
echo "TAG=$TAG" >> $GITHUB_ENV
set +e
git ls-remote "${{ secrets.DOC_PUSH_URL }}" -q
if [ "$?" -ne "0" ]; then exit 0; fi
set -e
git config --global user.name 'MFC Action'
git config --global user.email '<>'
git clone "${{ secrets.DOC_PUSH_URL }}" ../www
sed -i 's/"unit": "s\/iter"/"unit": "s"/g; s/cpu: /runtime: /g; s/iterations: /time steps: /g' ../www/documentation/data.js
sed -i "s/${GITHUB_SHA::7}/$TAG/g" ../www/documentation/data.js
git -C ../www add -A
git -C ../www commit -m "Docs @ ${GITHUB_SHA::7}" || true
git -C ../www push
- name: Polishing Results
run: |
TAG="${{ github.event.inputs.tag || github.ref_name }}"
echo "tag=$TAG" >> $GITHUB_OUTPUT
echo "TAG=$TAG" >> $GITHUB_ENV
set +e
git ls-remote "${{ secrets.DOC_PUSH_URL }}" -q
if [ "$?" -ne "0" ]; then exit 0; fi
set -e
git config --global user.name 'MFC Action'
git config --global user.email '<>'
git clone "${{ secrets.DOC_PUSH_URL }}" ../www
sed -i 's/"unit": "s\/iter"/"unit": "s"/g; s/cpu: /runtime: /g; s/iterations: /time steps: /g' ../www/documentation/data.js
sed -i "s/${GITHUB_SHA::7}/$TAG/g" ../www/documentation/data.js
git -C ../www add -A
git -C ../www commit -m "Docs @ ${GITHUB_SHA::7}" || true
for i in {1..5}; do
git -C ../www pull --rebase && git -C ../www push && break
sleep 5
done


- name: Print Logs
if: always()
run: |
cat pr/bench-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true
cat master/bench-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true

# All other runners (non-Phoenix) just run without special env
- name: Archive Logs (Frontier)
if: always() && matrix.cluster != 'phoenix'
uses: actions/upload-artifact@v4
with:
name: ${{ matrix.cluster }}-${{ matrix.device }}-${{ matrix.interface }}
path: |
pr/bench-${{ matrix.device }}-${{ matrix.interface }}.*
pr/build/benchmarks/*
master/bench-${{ matrix.device }}-${{ matrix.interface }}.*
master/build/benchmarks/*
1 change: 1 addition & 0 deletions .github/workflows/docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ jobs:
rm -rf ../www/*
mv build/install/docs/mfc/* ../www/
git -C ../www add -A
git -C ../www restore documentation/data.js
git -C ../www commit -m "Docs @ ${GITHUB_SHA::7}" || true
git -C ../www push

Expand Down
Loading
Loading