Skip to content
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

Report CBMC verification time #1247

Merged
merged 5 commits into from
Jun 8, 2022

Conversation

zhassan-aws
Copy link
Contributor

Description of changes:

Currently, there is no easy way to determine the time spent in the verification step (CBMC). What I typically do is to run kani or cargo kani with /usr/bin/time -p, but the reported time includes the time spent in compilation, codegen, verification, and processing of CBMC output. This PR adds reporting of the elapsed time for the verification step, which is needed when doing performance comparisons. The output looks like this:

SUMMARY: 
 ** 0 of 811 failed (20 unreachable)

VERIFICATION:- SUCCESSFUL


Verification Time: 3.579173s

Resolved issues:

Call-outs:

Testing:

  • How is this change tested? Added one test

  • Is this a refactor change? No

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@zhassan-aws zhassan-aws requested a review from a team as a code owner June 1, 2022 23:08
@adpaco-aws
Copy link
Contributor

CBMC reports runtimes for different processes (SSA conversion, solver, etc.), including decision procedure. Is that not good for some reason?

let _cbmc_result = self.run_redirect(cmd, &output_filename)?;
let elapsed = now.elapsed().as_secs_f32();
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any possible scenarios where there might be no time captured in the variable elapsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't think of a scenario.

@@ -51,6 +54,7 @@ impl KaniSession {
// the best possible fix is port to rust instead of using python, or getting more
// feedback than just exit status (or using a particular magic exit code?)
}
println!("Verification Time: {}s", elapsed);
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of non-termination, this would line would never be reached right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and also in case CBMC exited with an error.

@@ -51,6 +54,7 @@ impl KaniSession {
// the best possible fix is port to rust instead of using python, or getting more
// feedback than just exit status (or using a particular magic exit code?)
}
println!("Verification Time: {}s", elapsed);
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of non-termination, this would line would never be reached right?

let _cbmc_result = self.run_redirect(cmd, &output_filename)?;
let elapsed = now.elapsed().as_secs_f32();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why f32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the alternative you have in mind? Is it f64? If so, we probably don't need that much precision, but probably won't hurt much either.

// Copyright Kani Contributors
// SPDX-License-Identifier: Apache-2.0 OR MIT

// Checks that the time spent in CBMC is reported as "Verification time: <>s"
Copy link
Contributor

Choose a reason for hiding this comment

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

The actual check is just that there is a line that says Verification Time:, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I'll clarify the comment a bit.

@zhassan-aws
Copy link
Contributor Author

CBMC reports runtimes for different processes (SSA conversion, solver, etc.), including decision procedure. Is that not good for some reason?

Yes, there is no report of the overall CBMC runtime (which is necessary for performance comparisons). If I add up all the runtimes reported by CBMC on one example:

$ grep Runtime cbmc.log 
Runtime Symex: 0.289264s
Runtime Postprocess Equation: 0.00119202s
Runtime Convert SSA: 0.0894082s
Runtime Post-process: 0.00223362s
Runtime Solver: 0.0984715s
Runtime decision procedure: 0.188598s

they add up to less than 1 second, whereas the total CBMC runtime reported by the new line I added is more than 3 seconds:

Verification Time: 3.548905s

@tedinski
Copy link
Contributor

tedinski commented Jun 2, 2022

they add up to less than 1 second, whereas the total CBMC runtime

Heh. I wonder if we can get CBMC to add a feature that reports start/end timestamps instead of time.

It'd be nice to create the kind of trace diagrams that cargo build --timings gives.

@zhassan-aws
Copy link
Contributor Author

Heh. I wonder if we can get CBMC to add a feature that reports start/end timestamps instead of time.

It'd be nice to create the kind of trace diagrams that cargo build --timings gives.

That would be neat. I didn't know about --timings, but it would be a useful addition.

@zhassan-aws zhassan-aws merged commit adebe36 into model-checking:main Jun 8, 2022
@zhassan-aws zhassan-aws deleted the measure-cbmc-time branch June 8, 2022 19:23
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.

5 participants