Skip to content

Conversation

@stilscher
Copy link
Member

Closes #351

This PR contains the refinement for the incremental analysis which allows to reuse previous results for the unchanged nodes in changed functions. Instead of differentiating only between changed, unchanged, removed and added functions, changed functions can now be compared more detailed based on their control flow graph to determine the unchangedNodes, primObsoleteNodes and primNewNodes in the function. The unchanged nodes are tuples of old and new nodes that match and that only have incoming edges from other matching nodes. The primary obsolete nodes are nodes from the old CFG for which no match could be found and that suffice to be destabilized in an incremental analysis run. That means, that all changed nodes can be reached from the primary obsolete nodes. The primary new nodes are the equivalent to the primary obsolete nodes in the new CFG.

The changes of this PR include

  • configuration option for activating this refinement
  • change detection on control flow graphs for different versions of a function
  • extending the preparation for the incremental analysis in TD3 to handle the more detailed change info (it should also allow to activate refinement Incremental TD3: reluctant function destabilization #369 at the same time)
  • adapting the updateIds function to differentiate between unchanged and changed nodes in changed functions when updating the ids
  • extending the script for incremental test runs on repository to reset the incremental data when starting and also measure the total internal runtime

@stilscher stilscher marked this pull request as draft October 25, 2021 11:15
Copy link
Member

@jerhard jerhard left a comment

Choose a reason for hiding this comment

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

Added some comments on minor things. I did not have a deep look into the actual CFG-comparison yet. Maybe you can add some short comments there?

@sim642 sim642 self-requested a review October 25, 2021 13:03
@stilscher
Copy link
Member Author

I added comments for the compare algorithm and renaming as you suggested in the comments. Also I added the separate compareCIL module which uses the compare functions either from CompareCFG or CompareAST based on the configuration.

@stilscher stilscher marked this pull request as ready for review October 25, 2021 15:39
@sim642 sim642 added feature performance Analysis time, memory usage labels Oct 26, 2021
By that they get a new id and are reananalyzed for reluctant destabilization without any function-internal destabilization of primary obsolete nodes
@jerhard jerhard dismissed their stale review October 29, 2021 07:38

Changes requested in review were made.

ignore @@ Pretty.printf "test for %a\n" Node.pretty_trace (S.Var.node x);
solve x Widen;
if not (op (HM.find rho x) old_rho) then (
print_endline "Destabilization required...";
Copy link
Member

@jerhard jerhard Nov 2, 2021

Choose a reason for hiding this comment

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

Should this printout be there? Maybe use tracing instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This only extends the existing printouts during the incremental preparation in TD3 to make them more comprehensible in case of reluctant destabilization. I guess one should change all of them such that they are only output in verbose mode or use some existing tracing functions...

Copy link
Member

Choose a reason for hiding this comment

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

I think for the time being it's better if they aren't tracing, but also usable normally. It makes interpreting and debugging the interactive benchmark logs easier to see what happened and what didn't.
So making them verbose-only would be more reasonable, but since the rest are normally printed right now anyway, I suppose this can stay for now.

@stilscher stilscher merged commit b6f36ae into master Nov 2, 2021
@stilscher stilscher deleted the incremental/within-functions branch November 2, 2021 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature performance Analysis time, memory usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge incremental analysis improvements

4 participants