danmar
Pick job count dynamically when -j0 is used
Resolves a TODO for the -j argument, making it more similar to the behavior of tools like make and ninja. Instead of aborting when the user requests "zero jobs", pick a job count which is appropriate for the current system.
BTW I also made a patch for treating -j with no argument the same as -j0 (the way ninja and run-clang-tidy work). But in cases where -j is followed by a filename, it is necessary to separate the options from the args with --, so it's probably not good enough.
https://github.com/danmar/cppcheck/commit/62fa32409296d3c2b8991be792232c9ebb34942d
Thanks for your contribution.
This change was proposed before - see my previous comments on it here: https://github.com/danmar/cppcheck/pull/5326#discussion_r1294640814.
Thanks for the quick response. I had been wondering what the #5326 in d064f9c24 refers to, since the trac issue under that ID is unrelated; I get it now!
I came up with this PR because I am doing a C++ pipeline with a bunch of static analyzers and noticed that iwyu and cppcheck don't support this otherwise widespread option. So count me down as that one guy in 10 years :)
IMO anyone using -j0 is not going to be disappointed to get to 100% CPU load. Whether the feature is worth it with regard to the added maintenance burden, only you guys know.
If you decide that this is not a desirable feature, it might be good to replace the TODO.
making it more similar to the behavior of tools like make and ninja.
thanks I like to have similar behavior
Whether the feature is worth it with regard to the added maintenance burden, only you guys know.
I believe it's worth it.
IMO anyone using -j0 is not going to be disappointed to get to 100% CPU load.
In that other comment firewave said that we don't want to check all files at once.. and I agree with that.
@firewave you said that "std::thread::hardware_concurrency() might provide the desired information but it is implementation-dependent and any fallback would be extremely non-portable code which I would like to avoid." I don't have good knowledge about this. But I think the fallback to fail seems reasonable to me.. so this is not portable but at least it will start working on some platforms.
But my assumtion is that a non-zero value from std::thread::hardware_concurrency() is good to use. Do you know otherwise..
I have more comments on this but I am currently having issues with my hands (among other things) making it hard to do any work.
do you still have more opinions?
Yes. I keep forgetting about this...will reply later.
Sorry for the late reply.
-j0 is actually not that different from -j and only makes sense on a dedicated machine or in the CI.
On a desktop it does not make sense as this is most likely to overload your machine. Not as badly as -j but still. It will take CPU away from other applications and the system and will be disruptive. It should be limited to at most 80% of the available threads. But on my own experience even that might be too much depending on your setup and workload. As limiting it wouldn't do what -j0 is supposed to do, it would be very misleading to have that option at all.
So I do not think this should be added. This never having come up before also supports this.
On Linux and such you can easily get the desired value via nproc. On Windows it is unfortunately only possible to get the number of processors or cores but not the threads so there is no generic alternative.
If you decide that this is not a desirable feature, it might be good to replace the TODO.
Yeah - that TODO should have never made it into the code.
@pvutov do you actually intend to use this in CI? i.e. will this feature have some active use? Or did you mostly want to pick a simple task and contribute to Cppcheck?
and only makes sense on a dedicated machine or in the CI.
yes I agree.
So I do not think this should be added. This never having come up before also supports this.
I think that a pretty good argument to add it is to make cppcheck interface more similar to run-clang-tidy and make
I've already implemented my pipeline to call nproc, so I won't use it in CI. I submitted the PR because i was surprised to do the equivalent of make build -j; clang-tidy -j; clang-fomat -j; cppcheck -j; iwyu -j; coverity -j and notice that the flag is not universally supported.
For CI, you write the pipeline once so it is not a big deal to hardcode a more complicated command like cppcheck -j $(nproc) when the feature is missing. The impact of the feature there is just in removing a potential surprise and making the pipeline code simpler.
I think the more significant improvement is for the interactive workflow. @firewave does not like this use case but IMO it should be left to the user to know what they want. If the user calls with -j0 and their intent is clear, I think it's the wrong call to try to stop them. They are in a better position to decide whether the slowdown from make -j or whatever is tolerable for them or not.
@firewave does not like this use case but IMO it should be left to the user to know what they want.
It is not that I do not like the use case. First it is rare and second I personally experienced the downside of it.
The IDE I use introduced a default value for -j at some point with the maximum values and it took a several reports for that value to be lowered to 80% because it degraded the overall performance. And that is still not great - but to be fair that IDE is not really good at responsiveness and seem to lack some basic concepts on how an IDE is to behave (I really need to file a ticket about it).
But this seems to come down to the applications involved. Invoking a Cppcheck Visual Studio build via CMake might render your whole system unresponsive because they have an -j on three different levels and there appears to be no dependencies in-between these (and there also settings which are not being applied - another ticket I need to file). But if you build the project within the Visual Studio IDE this does not happen at all - even though the system is fully loaded as well.
So it is possible for the "interactive" use case are some additional things we need to address (maybe it is thread affinity or something else - I have not looked into what VS is doing).
But this makes me think of some different but related. That modern concept of E/P cores. There is OS level scheduling for this (which is being tweaked all the time) but I wonder how that would know that a random application like Cppcheck should use P cores. Are there system-level configurations or hints the application would provide? Something possibly to look into now that all three major CPU groups will use configuration coming end of July...