Conversation
…csson#718) The analysis time of the individual translation units was not measured, so it was not possible to tell which files or which analyzer made an analysis slow. Measure the time spent on every translation unit and summarize it per analyzer in the metadata file and in the analysis summary.
Contributor
|
Sorry, but I would close this PR, because it is being implemented in another PR: #5037 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #718
Problem
There is no way to tell how the analysis time is distributed. If an analysis is
slow, the user cannot find out which analyzer or which translation units are
responsible for it, so there is nothing concrete to optimize.
What was missing
The issue asks for five things, and two of them are already available today:
Analysis length: X sec) andstored in
metadata.jsonundertimestamps.with
CodeChecker analyze --file <path>.The remaining three - time per analyzer, average per translation unit, and the
slowest translation units - all need the same missing piece:
check()inanalysis_manager.pyanalyzes one translation unit per worker process, but itnever measures how long that takes. Its result tuple carries the return code,
the result file and the source file, but no timing, so nothing downstream can
aggregate it.
Fix
Measure the wall clock time of every translation unit in
check()and return itwith the rest of the result.
worker_result_handler()groups these per analyzerand stores the summary in
metadata.jsonunderanalyzer_statistics:The analysis summary also prints one line per analyzer:
Notes on the approach:
they are generated by the
analyzecommand and kept with the reports, so theycan be inspected in the terminal without running a server. No server, schema or
migration changes are needed - the new key is simply ignored by the store logic.
analysis runs.
analysis, since translation units are analyzed in parallel. This is documented.
skip_prefixesin theanalyze_and_parsetest, because the measured times differ from run to run. It removes only the
new lines, so the existing expected outputs are unchanged.
Testing
analyzer/tests/unit/test_analysis_time_statistics.py: thesummary calculation, the cap on the slowest translation units, aggregation per
analyzer into the metadata, skipped files being left out, and analyzers that
analyzed nothing.
test_analysis_time_statisticsintests/functional/analyze/test_analyze.py: runs a real analysis and checksboth the printed line and the contents of
metadata.json.helper functions - the wiring tests then fail and the calculation tests still
pass.
tests/unitandtests/functional/analyzeshow no new failures against aclean-tree baseline.
pycodestyleclean andpylint10.00/10 on every changed file.Out of scope / follow-ups
map_asynccallback (worker_result_handler)is swallowed and the analysis then waits on the pool's ~1 year timeout, so a
bug there hangs the analysis instead of failing. Worth handling separately.
skippedflag in the result tuple ofcheck()is alwaysFalse; thebranch handling it in
worker_result_handlerlooks vestigial.the
storecommand, are deliberately left out - the issue discussion arguesagainst putting this in the database.