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

Speedup clang tidy #3219

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

WeiqunZhang
Copy link
Member

We make ccache save a log file, from which we find out which files need a
rebuild. Instead of running clang-tidy on every file, we now only run it on
those files that had a miss in ccache. This significantly speeds up
clang-tidy in CIs.

Note that we treat clang-tidy warnings as errors in CIs. So if there is a
hit in the ccache's cache, the file is free of clang-tidy warnings. Also
note that we set CCACHE_EXTRAFILES to the clang-tidy configuration file. So
any changes to the config file will invalidate the cache as it should.

If a CI job successfully passes the compilation stage with ccache enabled,
but fails at the clang-tidy stage, the ccache's cache has been updated. One
might think this will cause issues because we now have files that have been
cached by ccache and meanwhile have failed the clang-tidy checks. However,
this is not an issue for Github CIs. This is because ccache's cache will not
be saved as a Github cache, if a CI fails.

Fix various clang-tidy warnings in Tests/. They were not reported before
because clang-tidy was not used on the tests.

@WeiqunZhang
Copy link
Member Author

Depend on #3218.

args = parser.parse_args()

fin = open(args.input, "r")
fout = open(args.output, "w")

Check warning

Code scanning / CodeQL

File is not always closed

File may not be closed if an exception is raised.
@WeiqunZhang
Copy link
Member Author

@yut23

@WeiqunZhang WeiqunZhang force-pushed the speedup_clang-tidy branch 7 times, most recently from 7380f91 to 0e27100 Compare March 28, 2023 01:55
We make ccache save a log file, from which we find out which files need a
rebuild. Instead of running clang-tidy on every file, we now only run it on
those files that had a miss in ccache. This significantly speeds up
clang-tidy in CIs.

Note that we treat clang-tidy warnings as errors in CIs. So if there is a
hit in the ccache's cache, the file is free of clang-tidy warnings. Also
note that we set CCACHE_EXTRAFILES to the clang-tidy configuration file. So
any changes to the config file will invalidate the cache as it should.

If a CI job successfully passes the compilation stage with ccache enabled,
but fails at the clang-tidy stage, the ccache's cache has been updated. One
might think this will cause issues because we now have files that have been
cached by ccache and meanwhile have failed the clang-tidy checks. However,
this is not an issue for Github CIs. This is because ccache's cache will not
be saved as a Github cache, if a CI fails.

Fix various clang-tidy warnings in Tests/. They were not reported before
because clang-tidy was not used on the tests.
@WeiqunZhang WeiqunZhang marked this pull request as ready for review March 29, 2023 20:24
@WeiqunZhang
Copy link
Member Author

I turned off EB in a 1D test, because we do not support EB in 1D.

@WeiqunZhang WeiqunZhang merged commit d781598 into AMReX-Codes:development Mar 30, 2023
@WeiqunZhang WeiqunZhang deleted the speedup_clang-tidy branch March 30, 2023 17:41
@ax3l ax3l added the cleaning label Apr 19, 2023
guj pushed a commit to guj/amrex that referenced this pull request Jul 13, 2023
We make ccache save a log file, from which we find out which files need
a
rebuild. Instead of running clang-tidy on every file, we now only run it
on
those files that had a miss in ccache. This significantly speeds up
clang-tidy in CIs.

Note that we treat clang-tidy warnings as errors in CIs. So if there is
a
hit in the ccache's cache, the file is free of clang-tidy warnings. Also
note that we set CCACHE_EXTRAFILES to the clang-tidy configuration file.
So
any changes to the config file will invalidate the cache as it should.

If a CI job successfully passes the compilation stage with ccache
enabled,
but fails at the clang-tidy stage, the ccache's cache has been updated.
One
might think this will cause issues because we now have files that have
been
cached by ccache and meanwhile have failed the clang-tidy checks.
However,
this is not an issue for Github CIs. This is because ccache's cache will
not
be saved as a Github cache, if a CI fails.

Fix various clang-tidy warnings in Tests/. They were not reported before
because clang-tidy was not used on the tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants