Skip to content

Conditional flyweight pattern for terms#692

Merged
marcoeilers merged 20 commits into
masterfrom
meilers_conditional_flyweight
Apr 19, 2023
Merged

Conditional flyweight pattern for terms#692
marcoeilers merged 20 commits into
masterfrom
meilers_conditional_flyweight

Conversation

@marcoeilers

@marcoeilers marcoeilers commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

This PR implements the flyweight pattern for Silicon terms in a way that can be turned on or off (depending on the value of Verifier.config.useFlyweight).
The motivation is that the flyweight pattern is required to use the Z3 API in a fast way (since translating the same term multiple times slows down the Z3 API massively for large examples with many terms, so they need to be cached, which is too slow unless flyweight is used) but using the flyweight pattern slows down Silicon when using Z3 via StdIO. Thus, Verifier.config.useFlyweight is set to true nly if Z3 is used via its API.

The PR borrows code and ideas from the flyweight pattern implementation by Fabian Bösiger, but does not use Scala macros to avoid IDE issues.

Instead, it uses

  • a trait ConditionalFlyweight that is implemented by all Term classes, which defines equality and hashCode either based on reference equality (if Verifier.config.useFlyweight is set) or otherwise on the values of a field equalityDefiningMembers, similar to the existing StructuralEquality trait.
  • a trait GeneralCondFlyweightFactory (and some more specific subtraits), to be implemented by companion objects of all Term classes, which contains a TrieMap to cache all existing instances of the class, and defines functionality for conditionally creating new instances or first checking for existing ones in the cache, again based on the value of Verifier.config.useFlyweight.

(Also, the PR fixes parameter handling in the Z3 API.)

@marcoeilers marcoeilers requested a review from mschwerhoff March 13, 2023 22:23
@marcoeilers marcoeilers requested review from mschwerhoff and removed request for mschwerhoff April 3, 2023 22:44
@marcoeilers

Copy link
Copy Markdown
Contributor Author

I hereby request a review from @mschwerhoff :)

@mschwerhoff mschwerhoff left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few minor comments, but no objections to merging this PR.

Comment thread src/main/scala/Config.scala
Comment thread src/main/scala/decider/Decider.scala
Comment thread src/main/scala/decider/Z3ProverAPI.scala Outdated
Comment thread src/main/scala/decider/Z3ProverAPI.scala
Comment thread src/main/scala/decider/Z3ProverAPI.scala
Comment thread src/main/scala/state/Terms.scala
Comment thread src/main/scala/state/Terms.scala Outdated
Comment thread src/main/scala/state/Terms.scala Outdated
Comment thread src/main/scala/state/Terms.scala Outdated
Comment thread src/main/scala/decider/TermToZ3APIConverter.scala
@mschwerhoff

Copy link
Copy Markdown
Contributor

I hereby request a review from @mschwerhoff :)

Ask, and it shall be given to you 😇

@marcoeilers marcoeilers merged commit fc8a8c9 into master Apr 19, 2023
@marcoeilers marcoeilers deleted the meilers_conditional_flyweight branch April 19, 2023 13:11
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