Skip to content

added command-line option --valueflow-max-iterations to control amount of valueflow iterations / also log debug warning when iterations are being exceeded - #4557

Merged
danmar merged 1 commit into
cppcheck-opensource:mainfrom
firewave:vf-max
Dec 20, 2022

Conversation

@firewave

@firewave firewave commented Oct 19, 2022

Copy link
Copy Markdown
Collaborator

This would allow the user to fully leverage ValueFlow (given it already has a reasonable run-time).

It is intentionally left-out of the documentation (for now) since it might lead to false positives like DISABLE_VALUEFLOW=1.

Regarding the debug message. This is not that different from other ValueFlow bailouts so we should track this. With it we can check in daca how many packages are actually exceeding the ValueFlow iterations.

I also have some improvements for the existing experimental timeout code but I will tackle that in a different PR. If you have ideas/suggestion for other tunables feel free to mention them.

…unt of valueflow iterations / also log debug warning when iterations are being exceeded
@firewave
firewave marked this pull request as ready for review October 19, 2022 11:42
@danmar

danmar commented Oct 20, 2022

Copy link
Copy Markdown
Collaborator

I have the feeling users would somehow be interested to have different settings for different files. Maybe a user normally wants to have 8 iterations, but then on some files valueflow is a big problem and then the user wants to have a low value for those.

@firewave

Copy link
Copy Markdown
Collaborator Author

That's possible. But without any data collected in daca we don't even know how many iterations are actually being performed. In our code we don't exceed it - otherwise the selfcheck would have failed.

@danmar

danmar commented Oct 22, 2022

Copy link
Copy Markdown
Collaborator

That's possible. But without any data collected in daca we don't even know how many iterations are actually being performed.

I envision that this will be a large feature that we don't implement quickly.

One possible workflow might be like this:

  • User thinks that normal Cppcheck analysis is too slow.
  • User runs the full Cppcheck analysis with some additional option; then CPU statistics for each file is generated.
  • Using the statistics the user can see how much time all valueflow iterations take. And can get a idea how much time he can save by limiting the number of iterations for certain selected files.
  • The user can configure how much valueflow cppcheck will run on each file.

@firewave
firewave marked this pull request as draft October 23, 2022 13:20
@firewave

Copy link
Copy Markdown
Collaborator Author

That's possible. But without any data collected in daca we don't even know how many iterations are actually being performed.

I envision that this will be a large feature that we don't implement quickly.

I think that would require a .cppcheck like config file - there's a ticket about that which I could not find.

Starting with actually exposing those tunable and providing feedback is a way too start. We have too many hard-coded things.

@firewave
firewave marked this pull request as ready for review December 20, 2022 15:35
@danmar

danmar commented Dec 20, 2022

Copy link
Copy Markdown
Collaborator

Well as a undocumented flag I can allow this.

There is a potential business deal that would make it possible to look considerably into performance. I have no idea when I will know if we get it or not.

@danmar
danmar merged commit b380fd2 into cppcheck-opensource:main Dec 20, 2022
@firewave
firewave deleted the vf-max branch December 20, 2022 20:04
@firewave

Copy link
Copy Markdown
Collaborator Author

Packages which exceed this: http://cppcheck1.osuosl.org:8000/head-valueFlowMaxIterations

I will add it to the main page with some upcoming daca-server PR.

@firewave

Copy link
Copy Markdown
Collaborator Author

I forgot to provide the filename that experiences this.

@firewave

firewave commented Dec 21, 2022

Copy link
Copy Markdown
Collaborator Author

I forgot to provide the filename that experiences this.

Fixed in #4664.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants