Skip to content
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.

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

Delete all in-tree benchmark infrastructure code. #18144

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

ScottTodd
Copy link
Member

@ScottTodd ScottTodd commented Aug 7, 2024

See the related RFC: https://groups.google.com/g/iree-discuss/c/n039Q0trkHk

Deleted components directly related to benchmark infrastructure:

  • Workflow files in .github/workflows/: benchmark triggering, e2e test artifact generation, benchmark compilation stats, benchmark execution, comment posting
  • Benchmark configuration and series definitions (mix of Python scripts, CMake generators, CMake files, etc.)
  • Documentation relating to benchmarks on pull requests / using GitHub Actions

Deleted components indirectly related to benchmark infrastructure:

  • Support scripts like build_tools/benchmarks/set_android_scaling_governor.sh (could be useful for developers, but doesn't need to live in-tree)
  • iree_run_module_test (was used to test correctness for some benchmark workloads but was not fully adopted)
  • Build presets in build_tools/cmake/ scripts (e.g. test, benchmark, benchmark-with-tracing): too complex, trying to steer away from build scripts and towards CMake presets or explicit workflows without that indirection

Future cleanup possibilities:

  • build_all.yml (no longer any references)
  • Docker files: android, nvidia, nvidia-bleeding-edge

Closes #7795
Closes #8631
Closes #10599
Closes #10928
Closes #11703
Closes #11922
Closes #11953
Closes #14993
Closes #15452
Closes #16057
Closes #16157
Closes #16396
Closes #16431
Closes #17749

@ScottTodd ScottTodd added infrastructure Relating to build systems, CI, or testing cleanup 🧹 infrastructure/benchmark Relating to benchmarking infrastructure labels Aug 7, 2024
@benvanik
Copy link
Collaborator

benvanik commented Aug 7, 2024

party

ScottTodd added a commit that referenced this pull request Aug 8, 2024
I'm trying to reduce the number of scripts that we have in
[`build_tools/cmake/`](https://github.com/iree-org/iree/tree/main/build_tools/cmake),
so changes like #17996 will be
easier to make with confidence.

This makes the `build_test_runtime` CI job no longer depend on these
scripts:
*
[`build_tools/cmake/build_runtime.sh`](https://github.com/iree-org/iree/blob/main/build_tools/cmake/build_runtime.sh)
* After this, the last use will be deleted in
#18144
*
[`build_tools/cmake/setup_build.sh`](https://github.com/iree-org/iree/blob/main/build_tools/cmake/setup_build.sh)
*
[`build_tools/cmake/setup_ccache.sh`](https://github.com/iree-org/iree/blob/main/build_tools/cmake/setup_ccache.sh)

These layers of scripts made it somewhat easier to have CI workflows
just run scripts, but the scripts grew branches over time that obscured
what was actually running. In many cases the commands are quite simple
and can just be inlined.

Other ideas for simplification:
* Use https://github.com/marketplace/actions/get-cmake to install Ninja
across platforms (with caching)
* Use https://github.com/marketplace/actions/run-cmake together with
CMake presets ("configure"/"build"/"test", possibly also "workflow")
* Reduce reliance on
[`build_tools/cmake/ctest_all.sh`](https://github.com/iree-org/iree/blob/main/build_tools/cmake/ctest_all.sh)
- that excludes some tests based on platform (e.g. Windows) and adds
exclude labels for tests that need special hardware (e.g. GPUs), but
ideally `ctest` would work out of the box without such a script

ci-exactly: build_test_runtime
@ScottTodd
Copy link
Member Author

Synced and added a list of issues this will close as no longer relevant. Getting ready to merge soon.

@ScottTodd ScottTodd marked this pull request as ready for review August 13, 2024 22:55
@ScottTodd ScottTodd requested a review from antiagainst as a code owner August 13, 2024 22:55
Copy link
Contributor

@antiagainst antiagainst left a comment

Choose a reason for hiding this comment

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

Here you go!! :D

@hanhanW
Copy link
Contributor

hanhanW commented Aug 14, 2024

So many issues are closed! +39 −23,393 is amazing.

@ScottTodd ScottTodd merged commit 9b05f17 into iree-org:main Aug 14, 2024
36 checks passed
@ScottTodd ScottTodd deleted the benchmarks-delete branch August 14, 2024 17:55
ScottTodd added a commit that referenced this pull request Aug 14, 2024
Follow-up to #18144. Related to
#15332.

* `build_all.yml` was used as the first step in multiple other
workflows. New workflows are using `pkgci_build_packages.yml` directly
or nightly releases. Workflows could also use historical artifacts from
`pkgci_build_packages.yml` if they want to use versions different from
the nightly releases.
* `android.Dockerfile` was used for Android builds and benchmarks. New
workflows install the NDK on demand without needing a large Dockerfile.
* `nvidia.Dockerfile` and `nvidia-bleeding-edge.Dockerfile` were used
for CUDA/Vulkan benchmarks. New workflows rely on the drivers and
software packages that are already installed on runners. We could have
workflows install on demand or add new Dockerfiles as needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment