Skip to content

ASV demo#487

Open
Micky774 wants to merge 6 commits intodevfrom
zain/asv-demo
Open

ASV demo#487
Micky774 wants to merge 6 commits intodevfrom
zain/asv-demo

Conversation

@Micky774
Copy link
Contributor

@Micky774 Micky774 commented Mar 16, 2026

Description

This PR is a port of #478 to ASV.

Benefits:

  1. Since ASV is a popular and mature OSS project, and is broadly adopted by other large communities (e.g. pandas, scikit-learn, numpy, etc.), there is a significantly reduced maintenance and development burden. It's already feature-rich and tested through the community, allowing us to make use of it and focus on the actual benchmark design rather than implementation detials.
  2. ASV accumulates results per commit on dev in Artifactory, so each CI run only benchmarks the
    current commit on an update to dev.
  3. Benchmark classes follow a well-documented convention (setup/time_*), making it easy for anyone familiar with ASV to add new benchmarks without understanding custom infrastructure. Similarly, people unfamiliar with ASV can quickly onboard with existing documentation.
  4. Failures are reported per parameter combination without crashing the suite.
  5. ASV stores results per commit as JSON in Artifactory, enabling asv publish to generate an HTML dashboard showing performance trends across the full commit history. This can be done statically via github pages in a separate repo for a permanent benchmark/regression dashboard.
  6. The ASV CI steps are ~75 lines total (restore/run/upload), with no custom run management.

Downsides:

  1. ASV relies on subprocess isolation per-config, which is very expensive for TE which has ~4-6s import time (resulting in minutes per benchmark, w/ 99.99% of time spent importing). This means that traditional ASV benchmarking can end up taking a while.
  2. ASV is a CI-first benchmarking project, which means that adapting it for local/development benchmarking requires additional infrastructure to maintain.
  3. Less initial flexibility in parsing results into e.g. custom analysis and visualizations -- though this can be remedied by allowing local (non-CI) runs to dump raw timings for custom handling.

Considerations

This PR comes with a helper script, and an adapter script to allow for direct same-process benchmarking (to avoid the subprocess TE import overhead) resulting in fast and efficient benchmarks. We could, if we wanted to, utilize that script for CI as well. This would greatly decrease the cost of benchmarking, but would be less robust and require more careful maintenance. For the most part, this maintenance cost isn't that great, since ASV only has a couple releases a year, and is a stable project, meaning we can pin our version and not need to worry about API/format changes.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Adds ASV benchmarks
  • Updates CI to read/generate/write ASV results to artifactory
  • Adds README.md for documentation

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@Micky774 Micky774 marked this pull request as ready for review March 17, 2026 13:58
@Micky774
Copy link
Contributor Author

Note the CI failure is unrelated

@Micky774
Copy link
Contributor Author

I've added a helper script like @alextmagro had suggested, as well as corresponding documentation to the README.md.

EOF
)"

- name: Restore previous ASV results
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think benchmarks should go separate workflow from CI. I.e. these microbenchmarks and ones that are already run with CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will doing so require a separate TE build and setup? I added it here so that we'd piggy-back off of already running CI.

@@ -0,0 +1,16 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it need to be in root of TE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I've updated it


# Derive a stable machine name from the runner label
case "${RUNNER_NAME}" in
linux-te-mi325*) MACHINE_NAME="mi325" ;;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need it if results are uploaded with just matrix.runner name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, my understanding is that the matrix.runner name is not 1-1 with the underlying system, i.e. different systems with different machine names can be part of a pool with the same runner name. ASV by default stores results by machine name. Here, we are manually specifying a generic machine name indexed by gpu arch so that each e.g. mi325 runner will store its results in a compatible way.

Ideally, we have dedicated machines for benchmarking (since this would likely be every commit or nightly even), but that's a constraint we'll need to discuss.

set -ex
pip install asv
cd /workspace
asv machine --yes --machine "$MACHINE_NAME"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it re-register machine if it exists already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's registered in the container so it's transient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants