Skip to content

CBMC/JBMC --cover: only store traces with --trace to avoid memory exhaustion #5714

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

Merged
merged 3 commits into from
Jan 21, 2021

Conversation

tautschnig
Copy link
Collaborator

Not everyone needs test inputs, especially when programs lack INPUT
instructions anyway. This helped me avoid running out of memory on a
768GB system for a particular verification problem. The same problem can
now be run with just 75GB of memory.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@codecov
Copy link

codecov bot commented Jan 3, 2021

Codecov Report

Merging #5714 (87e8440) into develop (d613b01) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5714      +/-   ##
===========================================
- Coverage    69.67%   69.66%   -0.02%     
===========================================
  Files         1248     1248              
  Lines       100836   100854      +18     
===========================================
  Hits         70262    70262              
- Misses       30574    30592      +18     
Flag Coverage Δ
cproversmt2 43.37% <70.83%> (-0.06%) ⬇️
regression 66.64% <100.00%> (-0.02%) ⬇️
unit 32.20% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/goto-checker/goto_trace_storage.h 100.00% <ø> (ø)
src/goto-instrument/cover.h 100.00% <ø> (ø)
src/goto-programs/goto_trace.h 100.00% <ø> (ø)
src/cbmc/cbmc_parse_options.cpp 77.67% <100.00%> (+0.04%) ⬆️
...-checker/cover_goals_verifier_with_trace_storage.h 93.33% <100.00%> (+1.66%) ⬆️
src/goto-checker/goto_trace_storage.cpp 100.00% <100.00%> (ø)
src/goto-instrument/cover.cpp 85.41% <100.00%> (+0.10%) ⬆️
src/goto-programs/goto_trace.cpp 81.40% <100.00%> (+0.53%) ⬆️
src/cbmc/c_test_input_generator.cpp 60.00% <0.00%> (-30.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d613b01...87e8440. Read the comment docs.

@tautschnig tautschnig self-assigned this Jan 3, 2021
Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good workaround in the meanwhile. Storing traces and inputs should be decoupled, though, I think.
Besides, I should probably revive the PR that enables trace output as traces are produced rather than storing them at all (but that's quite a UI change which needs to be phased in properly).

@@ -1,6 +1,6 @@
CORE
main.c
--xml-ui --cover branch
--xml-ui --cover branch --trace

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this is broken somehow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was broken as the code generating XML output is used for both trace and test-suite output. I have therefore changed the patch to introduce a new command-line option "show-test-suite."

Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the performance numbers this sounds very much like the kind of thing we want and I am in favour of not storing things we don't need. But I am a little confused over the use-case for this. --cover creates a bunch of assertions for the relevant coverage metric and then solves for all of them. Then, orthogonally(? is this what @peterschrammel is suggesting?) we could output for each location:

A. reached / unreachable.
B. inputs that would cause the relevant path to be taken.
C. the execution trace that reaches each one.

So this is only fro case C? Are there users of this? Also, can't we output these as we compute them? I'm not sure I see why they need to be stored at all.

goto traces consume a lot of memory, and re-allocating them for each
incremental addition of a goto trace does not seem to be a good
approach. This is at the expense of more costly indexed access, which
now requires iterator increments.
@tautschnig
Copy link
Collaborator Author

@peterschrammel @martin-cs I'd appreciate you taking another look at this for I've made substantial changes over the version you previously reviewed/approved:

  1. I've introduced merge_irept to effectively compress the amount of memory consumed by a trace. This alone would have solved my out-of-memory problem as I've tested, but it seems the computational cost of building all the traces is still substantial (and often unnecessary).
  2. Instead of using (abusing?) the "trace" command-line option, I've instead added a new option "show-test-suite."

It may still be desirable to print test inputs as-you-go, but that's not something on my priority queue (as I don't care about those inputs values at the moment).

A goto trace includes freshly constructed expressions, which thus lack
sharing. This results in excessive memory use when storing multiple
traces, as is done for coverage computation. On a particular benchmark,
coverage computation previously ran out of memory at 768 GB. This same
benchmark can now be run with just 80 GB of memory.
Not everyone needs test inputs, especially when programs lack INPUT
instructions anyway. Test inputs are found via goto traces, and
computing those is expensive in both time and memory required. On one
benchmark, not building goto traces reduced the overall time from 36000
seconds to 2800 seconds (a saving of 92%).
@tautschnig tautschnig merged commit 4820ec3 into diffblue:develop Jan 21, 2021
@tautschnig tautschnig deleted the cover-trace branch January 21, 2021 22:59
@martin-cs
Copy link
Collaborator

@tautschnig Apologies for the lag; first week of term. I'm guessing by the merge that you are happy with this.

@tautschnig tautschnig mentioned this pull request Mar 25, 2024
5 tasks
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.

4 participants