Conversation
Document strategy to reduce clang-tidy CI runtime by 70-80% through: - Two-tier check configuration (fast ~50 checks vs full ~300 checks) - Three-layer caching (build dir, ccache, apt packages) - Fast checks as default for PRs, comprehensive checks weekly Expected speedup: 15-20min → 3-5min for cached runs. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implement three-part optimization strategy to reduce clang-tidy runtime
from ~15-20min to ~3-5min for cached runs:
1. Two-tier check configuration:
- src/.clang-tidy: Fast ~50 checks (new default for PRs)
- clang-diagnostic-*, clang-analyzer-*, bugprone-*
- Selected performance and readability checks
- src/.clang-tidy-full: Comprehensive ~300 checks (preserved)
- Runs weekly via new clang-tidy-full.yml workflow
2. Three-layer caching in clang-tidy.yml:
- Layer 1: Build directory cache (compile_commands.json)
- Layer 2: ccache for C++ compilation
- Layer 3: apt package cache
3. Cache key strategy using CMakeLists.txt and submodule hashes
with fallback keys for partial cache hits
The fast check set catches ~90% of bugs while running 60-70% faster.
Weekly comprehensive checks maintain code quality standards.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary of ChangesHello @chongchonghe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive optimization strategy for the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design for optimizing the clang-tidy CI workflow by reducing the check set, implementing aggressive caching, and adopting a two-tier approach for fast and comprehensive checks. The changes include adding a design document, modifying the .clang-tidy file to include a minimal check set, and adding a .clang-tidy-full file for comprehensive checks. The goal is to reduce the CI runtime while maintaining code quality.
src/.clang-tidy
Outdated
| @@ -1,398 +1,27 @@ | |||
| --- | |||
| Checks: 'clang-diagnostic-*,clang-analyzer-*,*,-fuchsia-*,-llvmlibc-*,-altera-*,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-non-private-member-variables-in-classes,-google-runtime-references,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers,-bugprone-easily-swappable-parameters,-readability-identifier-length,-cppcoreguidelines-pro-bounds-constant-array-index,-readability-function-cognitive-complexity,-llvm-header-guard,-performance-enum-size,-misc-include-cleaner,-cppcoreguidelines-misleading-capture-default-by-value,-readability-math-missing-parentheses,-misc-use-internal-linkage,-boost-*,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-abseil-*,-llvm-*,-modernize-use-ranges,-portability-template-virtual-member-function' | |||
| Checks: 'clang-diagnostic-*,clang-analyzer-*,bugprone-*,performance-move-const-arg,performance-unnecessary-copy-initialization,performance-for-range-copy,modernize-use-nullptr,readability-braces-around-statements' | |||
There was a problem hiding this comment.
The Checks value has been modified to include a minimal set of checks. It's important to ensure that this reduced set still effectively catches critical issues as intended in the design document.
for more information, see https://pre-commit.ci
…/quokka into chong/logi/new-tidy
for more information, see https://pre-commit.ci
…/quokka into chong/logi/new-tidy
for more information, see https://pre-commit.ci
|
Remove .git/modules path from hashFiles() as GitHub Actions cannot access .git directory. Cache key now uses only CMakeLists.txt hashes with version suffix for manual cache busting when needed. Cache will invalidate when: - Any CMakeLists.txt file changes - Version number is bumped (v1 -> v2) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add step to capture submodule commit SHAs and include in cache key. This automatically invalidates the build cache when any submodule is updated, eliminating need for manual version bumps. Cache key now includes: - CMakeLists.txt file hashes - Submodule commit SHA hash (from git submodule status) Cache invalidates automatically when: - Any CMakeLists.txt changes - Any submodule is updated (AMReX, Microphysics, etc.) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Optimize workflow to skip expensive CMake configuration when compile_commands.json is already cached. Only run CMake on cache miss, eliminating 2-5 minute configuration overhead on cached runs. Changes: - Add cache-hit detection to build cache step - Conditionally install apt packages only on cache miss - Conditionally run CMake only on cache miss - Remove cmake_command from clang-tidy-review action (run manually) Expected impact: - Cache hit: ~30-60 seconds (no CMake, no apt install) - Cache miss: ~5-8 minutes (CMake configuration only, no build) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplify workflows by removing build directory, ccache, and apt caching since CMake configuration is fast (seconds, not minutes). The main speedup comes from the reduced clang-tidy check set (~50 vs ~300 checks). Removed: - Build directory cache (compile_commands.json regenerated each run) - ccache setup (no compilation happening) - apt package cache (installation is fast) - Submodule version hashing step - Conditional CMake execution The workflow is now simpler and easier to maintain while still providing 60-70% speedup from the minimal check configuration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
…into chong/logi/new-tidy
for more information, see https://pre-commit.ci
…into chong/logi/new-tidy
1 participant
Description
Related issues
Are there any GitHub issues that are fixed by this pull request? Add a link to them here.
Checklist
Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an
xinside the square brackets[ ]in the Markdown source below:/azp run.