Parallelize across files

[mccrearyp]

I'm using a workflow in github that uses this project and its been working great, however it takes a long time to run. It looks like it processes the files serially one at a time. Is it possible to do something similar to a "-j" flag like clang-tidy supports to lint and gather the output from multiple files simultaneously so people have the option to allocate more jobs and reduce the time it takes the workflow to run?

Here is an example of the time it took in our CI:

When I ran clang-tidy directly with -j20 it took about 3-4 minutes.

[2bndy5]

TL;DR - Its on my radar, but not a high priority without community contributions.

I have been considering migrating to async ops, but I haven't experimented with it yet.

So far, the biggest slow-down seems to be when running clang-tidy (despite file size or diagnostics used). I know we could run clang-tidy on a batch of files at once, but that makes parsing clang-tidy's output much more complex (and risks making the CI logs confusing).

My first concern is the ordering of the logs. Processing things serially has made organizing the logs easier, but processing them in parallel will require large buffers for logs (that get flushed to stdout upon completion). This shall not be scoffed at because comprehensible/legible logs have proved invaluable when troubleshooting.

Secondly, Windows does impose limits about how many processes can access open files. This is a concern when running analysis in parallel using both clang-tidy and clang-format on the same set of files.

PS - I just did a complete refactor of the python code base. In addition to a new beta feature (PR review suggestions), you might see a small speed up in the upcoming cpp-linter-action v2.9.0 (or cpp-linter v1.7.1 if using the python pkg directly). Although, the PR review feature will definitely make runtime much longer...

---

FWIW, I've been working on a rust port of this pkg: cpp_linter_rs. Using rust as a backend will provide a better foundation (compared to CPython) to run tasks in parallel.

[mccrearyp]

Yea I was considering making the changes myself and you're right that it is not a trivial task. It would require making each clang-tidy process output into a separate temporary file and then aggregating the results from the files or using a command like parallel which isn't really portable (not a concern for me, but not ideal for a project like this that has a broad audience).

[2bndy5]

making each clang-tidy process output into a separate temporary file and then aggregating

Python's file read/write ops are not the most timely. In the recent refactor, I switch from using temporary files to using buffers and it improved the execution speed significantly; enabling cpp-linter's verbosity will generate temporary files (useful for debugging) located in .cpp-linter_cache folder. Unfortunately, using buffers is more memory intensive (especially with python) and further complicates implementing actual parallelism.

[2bndy5]

When I ran clang-tidy directly with -j20 it took about 3-4 minutes.

I don't see this -j option in clang-tidy v16.0.6 (on windows). When I try it, I get an error:

clang-tidy -p build -j2 src/demo.cpp src/demo.hpp   
error: clang-tidy.exe: Unknown command line argument '-j2'.  Try: 'C:\Program Files\LLVM\bin\clang-tidy.exe --help'
clang-tidy.exe: Did you mean '-h'?

@mccrearyp What version of clang-tidy are you using? Are you using some clang plugin? Is the option specific to Linux/Unix builds?

[2bndy5]

the -j feature is CMake, not clang-tidy

That's what I suspected. I think CMake's -j option is specific to whatever build system is being used (ninja, makefile, etc), not static analysis tools.

So, just to be clear, running clang-tidy on a batch of files does not imply parallelism, but it does reduce whatever startup ops that clang-tidy does when invoked from the shell.

Weather or not clang-tidy runs analysis in parallel on a batch of files cannot be controlled externally, as it seems to be an implementation detail.

Now the real question is: Which is faster? Running clang-tidy on a batch of files at once? Or running clang-tidy on each file in separate threads?

[mccrearyp]

I don't think my lint setup is batching files into clang-tidy. It looks like CMake runs a clang-tidy process per file. When I do cmake --build . -j 20 it actually runs multiple instances of clang-tidy in separate processes, I can see it in top:

For one of my recent changes that touched 100 files I got all the lint output from CMake in about 8 minutes while the lint workflow in github took about 1hr 23 minutes. Technically the lint output from CMake runs on every single file even if it wasn't modified, but this change was big enough to make it a close to an equal comparison against the github workflow which I have configured to only lint changed files.

It seems like running a clang-tidy instance per file and allowing multiple instances running simultaneously across multiple cores is the way to go.

[2bndy5]

Interesting! Thanks for the useful insight! 🚀

I was planning on having separate branches to implement both multi-threaded and batch-file approaches. Looks like I should start with multi-threaded first.

Again, I'm not sure when I can get into implementing/experimenting with this. My past experience with python's multi-threading libs haven't been "joyful", and CPython's GIL ultimately stopped me from achieving some semblance of true concurrency. I'm more excited to use rust on the backend for multi-threading since it would not be limited by CPython's GIL.

[mccrearyp]

It's been a while since I've touched Python, but when I get a moment I'll take a look and see if I can find some low hanging fruit that could produce a serviceable solution. If you're open to contribution I can try putting together a PR. I haven't done much development on projects that focus on portability and its been a few years since I've done anything meaningful in Python so I might just make more of a mess than its worth, lol.

[2bndy5]

I forgot about python's async lib. It was created shortly after my last attempt at using python's threading lib.

[2bndy5]

Bad news from clang-format --help

Narrowing clang-format analysis via line filter(s) is not natively supported (as it is in clang-tidy) on a batch of files.

 --lines=<string>               - <start line>:<end line> - format a range of
                                   lines (both 1-based).
                                   Multiple ranges can be formatted by specifying
                                   several -lines arguments.
                                   Can't be used with -offset and -length.
                                   Can only be used with one input file.

Fortunately, clang-format is pretty light weight compared to clang-tidy...

[2bndy5]

Parsing clang-format's XML output for a batch of files will be more complicated as well. Applying a line filter to a batch of files will require us to roll our own implementation.

Sample XML output from clang-format on a batch of 2 files

<?xml version='1.0'?>
<replacements xml:space='preserve' incomplete_format='false'>
<replacement offset='100' length='5'>&#10;&#10;</replacement>
<replacement offset='115' length='0'>&#10;</replacement>
<replacement offset='130' length='1'>&#10;        </replacement>
<replacement offset='137' length='7'>&#10;&#10;    </replacement>
<replacement offset='169' length='9'>&#10;&#10;    </replacement>
<replacement offset='187' length='0'>&#10;</replacement>
</replacements>
<?xml version='1.0'?>
<replacements xml:space='preserve' incomplete_format='false'>
<replacement offset='12' length='4'>&#10;&#10;</replacement>
<replacement offset='74' length='1'>&#10;        </replacement>
<replacement offset='76' length='0'> </replacement>
<replacement offset='83' length='0'>&#10;        </replacement>
<replacement offset='98' length='0'>&#10;    </replacement>
<replacement offset='99' length='0'>&#10;    </replacement>
<replacement offset='100' length='6'>&#10;&#10;</replacement>
<replacement offset='122' length='1'></replacement>
<replacement offset='124' length='0'> </replacement>
<replacement offset='139' length='1'></replacement>
<replacement offset='141' length='0'> </replacement>
<replacement offset='145' length='0'> </replacement>
<replacement offset='146' length='0'> </replacement>
<replacement offset='160' length='0'> </replacement>
<replacement offset='164' length='19'>&#10;&#10;</replacement>
<replacement offset='198' length='1'> </replacement>
<replacement offset='216' length='2'>&#10;</replacement>
</replacements>

Notice though, that there is no filename mentioned in the XML output. We'll have to assume that the order of XML documents corresponds to the order of specified input files.

---

Getting suggested fixes from clang-format can be done using a batch of files with clang-format's -i option (modify file in place instead of showing changes in stdout). This is similar to how we are currently getting suggested fixes from clang-tidy as clang-tidy does not use stdout to show modifications.

[2bndy5]

Big thanks to @jnooree for solving the challenges that this request presented. 💯 🎉 🥇

Parallelism is now implemented in cpp-linter-action v2.11.0 (& cpp-linter python package v1.8.0).

[2bndy5]

Benchmarking on the cpp-linter package repo (which has small sample sets of cached C/C++ sources for testing) showed ~1.96x speed up (on my 8-core i7 CPU). We're expecting even better performance in github runners though.

@mccrearyp let us know if you experience has been improved.

[mccrearyp]

Yes, it is much faster now, thank you!

[mccrearyp]

Finally had a refactor PR that touched a lot of files and could give some data behind this improvement:

May not be an apples to apples comparison (mainly that the annotate only does lines changed), but this action now significantly outpaces my CMake lint build which it wasn't able to do before.

So in my case where I have a lot of cores available we're actually getting 3.5x improvement. I suspect if we allocated more than 20 cores per job it could go even faster in these large scale changes.

Thanks again for getting this implemented!