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

Optional Snapshot Generation in Consume #879

Merged

Conversation

mlimbeck
Copy link
Contributor

During the execution of certain statements, consume generates snapshots that are unused. For consuming QP-Chunks, this results in unnecessary snapshot map definitions being triggered, increasing the number of quantifier instantiations and negatively impacting performance.

To address this, snapshot generation is now prevented for these cases. While the primary performance improvement applies to consuming QP-Chunks, the change is also applied to consuming non-QP-Chunks to ensure consistent behavior across all scenarios.

Example:

var rs: Set[Ref]
var x: Ref
var val: Int
assume x in rs
inhale forall r: Ref :: r in rs ==> acc(r.f)

val := x.f // This will trigger all snapshot map definitions related to x.f.

exhale forall r: Ref :: r in rs ==> acc(r.f, 1/3) 
exhale forall r: Ref :: r in rs ==> acc(r.f, 1/3) 
// The exhales will create new snapshot map definitions which are never needed but still triggered.

Affected Statements:

Field assignments
Method calls (consume of pres)
Exhale statements
Assert statements
While loops (consume of invariants)

@mlimbeck mlimbeck marked this pull request as ready for review November 26, 2024 16:49
@jcp19
Copy link
Contributor

jcp19 commented Nov 26, 2024

I wonder, did you benchmark this on sth like scion or sth generated by Prusti? I wonder what are the implications in performance (and potentially completeness)

@marcoeilers
Copy link
Contributor

Completeness should not be impacted in any way, the snapshots are only omitted when they definitely won't be used.

Copy link
Contributor

@marcoeilers marcoeilers left a comment

Choose a reason for hiding this comment

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

Thanks!
I marked three things, everything else looks fine to me.

src/main/scala/rules/Consumer.scala Show resolved Hide resolved
src/main/scala/rules/ChunkSupporter.scala Outdated Show resolved Hide resolved
src/main/scala/rules/MoreCompleteExhaleSupporter.scala Outdated Show resolved Hide resolved
@mlimbeck
Copy link
Contributor Author

I wonder, did you benchmark this on sth like scion or sth generated by Prusti? I wonder what are the implications in performance (and potentially completeness)

For the moment there are no measurable benefits for Scion. However, I expect it will improve the performance of run().

As an illustrative example, consider a case where we sum over n arrays. The following table demonstrates how the number of quantifier instantiations is reduced when using this approach:

Arrays with snapshots without snapshots
5 2.556 1.876
10 25.700 16.443
15 77.481 59.156

Example Code

method sum(a0: IArray, n: Int) returns (res:Int)
requires forall j: Int :: {loc(a0, j)}  0 <= j && j < n ==> acc(loc(a0, j).val, 1/2)
requires len(a0) == n
ensures forall j: Int :: {loc(a0, j)}  0 <= j && j < n ==> acc(loc(a0, j).val, 1/2)
ensures len(a0) == n
{
	var i: Int
	i := 0 
	res := 0 
	while(i < n )
        invariant forall j: Int :: {loc(a0, j)}  0 <= j && j < n ==> acc(loc(a0, j).val, 1/2)
        invariant len(a0) == n
        invariant  0 <= i && i <= n
        {
	   res := res + loc(a0, i).val
	    i := i + 1
      }
}

@marcoeilers marcoeilers merged commit 761d4dd into viperproject:master Nov 29, 2024
2 checks passed
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.

3 participants