Skip to content

Conversation

jezhiggins
Copy link
Contributor

@jezhiggins jezhiggins commented Aug 11, 2021

This PR reduces the number of times the operands in a constant_value expression are reevaluated. Usually, this overhead is small but for nested expressions this can result in excessive runtimes.

The motivating example has the form

(
 (l >= mh.s ? 1 : 0)
&(
  (
   (mh.s >= s ? 1 : 0) 
  &(
    (
     (n >= mh.t ? 1 : 0) 
    &(
      (
       (mh.t >= o ? 1 : 0) 
      &(
        (
         ((int)mh.to == (int)nt ? 0 : 1) 
        &((int)mh.su == (int)ns ? 0 : 1)
        ) == 0 ? 0 : 1
       )
      ) == 0 ? 0 : 1
     )
    ) == 0 ? 0 : 1
   )
  ) == 0 ? 0 : 1
 )
) == 0

This doesn't look particularly difficult, but evaluation time for the goto-program in question was close to two hours. With these changes evaluation time is approximately two tenths of a second (usual caveats - as reported by time on my machine, etc).

@jezhiggins jezhiggins changed the title VSD - constant evaluation speedup VSD - constant expression evaluation speedup Aug 11, 2021
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.

Looks good so far. A test case would be good.

@jezhiggins jezhiggins force-pushed the vsd-eval-within-our-lifetime branch 2 times, most recently from cfde5e3 to 9b7f4ff Compare August 11, 2021 22:46
@codecov
Copy link

codecov bot commented Aug 11, 2021

Codecov Report

Merging #6290 (bec4cb6) into develop (2f15ccf) will increase coverage by 0.00%.
The diff coverage is 97.36%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6290   +/-   ##
========================================
  Coverage    75.96%   75.96%           
========================================
  Files         1508     1508           
  Lines       163282   163292   +10     
========================================
+ Hits        124040   124052   +12     
+ Misses       39242    39240    -2     
Impacted Files Coverage Δ
...ses/variable-sensitivity/abstract_value_object.cpp 96.21% <96.66%> (-0.24%) ⬇️
src/goto-analyzer/static_verifier.cpp 76.36% <100.00%> (-0.22%) ⬇️
src/linking/linking.cpp 58.67% <100.00%> (+0.06%) ⬆️
src/solvers/flattening/arrays.cpp 80.45% <0.00%> (-2.84%) ⬇️
src/goto-programs/goto_program.cpp 69.84% <0.00%> (+1.36%) ⬆️
src/goto-programs/link_goto_model.cpp 86.44% <0.00%> (+8.47%) ⬆️

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 3092e7b...bec4cb6. Read the comment docs.

@jezhiggins jezhiggins force-pushed the vsd-eval-within-our-lifetime branch from 9b7f4ff to 25b91df Compare August 12, 2021 08:00
@jezhiggins jezhiggins force-pushed the vsd-eval-within-our-lifetime branch from 25b91df to c037f34 Compare August 12, 2021 08:07
@jezhiggins jezhiggins marked this pull request as ready for review August 12, 2021 09:49
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.

Great; thank you!

--
This expression is derived from a real system under test. Due to inadvertent reevaluation of expression operands, all expression evaluations had geometric complexity (even more so in the presence of TOP values). This particular expression took nearly two hours to evaluate. Reworking to prevent repeated reevaluation brings this down to a few tenths.
See https://github.com/diffblue/cbmc/pull/6290
If this test appears to hang, the constant_evaluator is probably your first port of call.
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️

@martin-cs martin-cs merged commit dbeb187 into diffblue:develop Aug 12, 2021
@jezhiggins jezhiggins deleted the vsd-eval-within-our-lifetime branch August 12, 2021 10:39
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