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

Record the SMT input size for each split #938

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

keyboardDrummer
Copy link
Collaborator

@keyboardDrummer keyboardDrummer commented Aug 14, 2024

Changes

Record the SMT input string size for each split, which can be used to detect regressions in encoding performance

Testing

Updated existing test xml.bpl

@keyboardDrummer keyboardDrummer enabled auto-merge (squash) August 14, 2024 11:41
Copy link
Collaborator

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This seems like a useful thing to have. I'd prefer to have the size increment and Send calls paired in a separate method, though I don't feel strongly about that.

@@ -647,6 +653,7 @@ protected override void Send(string s, bool isCommon)

if (Process != null)
{
SentSize += s.Length;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be lovely if this could only be done from a wrapper around Send, to ensure that it's always incremented. It would be even nicer if the reset to size 0 could be encapsulated somewhere, too, though I remember from working on this code that there isn't a clear place to put it at the moment.

Copy link
Collaborator

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This looks good overall, but it looks like there are some minor test fixes needed.

@@ -11,11 +11,11 @@
// CHECK: \<method name="ExampleWithSplits" startTime=".*"\>
// CHECK: \<assertionBatch number="1" iteration="0" startTime=".*"\>
// CHECK: \<assertion file="xml.bpl" line="25" column="3" /\>
// CHECK: \<conclusion duration=".*" outcome="valid" resourceCount=".*" /\>
// CHECK: \<conclusion duration=".*" outcome="valid" resourceCount=".*" smtInputSize="601" /\>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like these values need updating again.

@keyboardDrummer
Copy link
Collaborator Author

Currently sometimes (declare-fun ControlFlow (Int Int) Int) gets put into common and sometimes it is not, which can change the numbers. Should find a way so it's always included

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