Skip to content

Add cached interpretations and use for typeof - #716

Merged
eb8680 merged 26 commits into
masterfrom
jf-cache-intp
Jul 24, 2026
Merged

Add cached interpretations and use for typeof#716
eb8680 merged 26 commits into
masterfrom
jf-cache-intp

Conversation

@jfeser

@jfeser jfeser commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Adds a notion of a cached interpretation (these interpretations handle the internal operation _is_memoized). When evaluating a term under a cached interpretation, we set a cache attribute on the term that we can reuse for later evaluations under the same interpretation.

Blocked by #713 (only for the benchmark)

Before:

---------------------------------------------------- benchmark: 1 tests ----------------------------------------------------
Name (time in ms)                    Min       Max     Mean   StdDev   Median     IQR  Outliers      OPS  Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------
test_bench_term_construction     60.1234  127.2249  65.1138  16.5825  60.9053  0.8398       1;2  15.3577      16           1
----------------------------------------------------------------------------------------------------------------------------

After:

-------------------------------------------------- benchmark: 1 tests -------------------------------------------------
Name (time in ms)                   Min     Max    Mean  StdDev  Median     IQR  Outliers       OPS  Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------
test_bench_term_construction     4.1599  5.0849  4.4263  0.1847  4.3989  0.2841      35;1  225.9233     111           1
-----------------------------------------------------------------------------------------------------------------------

Open design questions:

  • The cache needs to distinguish non-equivalent interpretations. It does this with object identity. We could use a stronger comparison operation and get more cache hits. For example, coproduct({}, i) != i in the current implementation.
  • Whether an interpretation is cached is determined by whether it handles an operation. This means that coproducts with this interpretation will also be cached. Do we want cachability to inherit in this way?

Previous attempt: #252

@jfeser jfeser added the blocked label Jul 17, 2026
@jfeser
jfeser requested a review from eb8680 July 17, 2026 20:18
@jfeser jfeser removed the blocked label Jul 20, 2026
@jfeser
jfeser marked this pull request as draft July 20, 2026 18:09
@jfeser

jfeser commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

#594 is related; it adds a cache that persists for a single evaluate call. This approach is orthogonal. This PR does not improve the evaluation of terms with sharing, and #594 does not reduce the complexity of defdata.

@eb8680 eb8680 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.

Looks like a nice improvement, I just have a couple of small comments.

Also, what happened to removing internals.product_n?

Comment thread effectful/ops/semantics.py Outdated
Comment thread effectful/ops/semantics.py Outdated
return frozenset(intp.items())


def _term_cache(expr: Term) -> dict[object, object] | None:

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.

Would it be cleaner to define this as a functools.cached_property of Term?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This version avoids the need to update implementers of Term, but a cached property is cleaner.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think since we have a variety of internal and external implementers of Term, some of which don't subclass it, we should stick with the current implementation.

@jfeser

jfeser commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

I found new uses for product_n in #719.

Comment thread effectful/ops/types.py Outdated
Comment thread effectful/ops/syntax.py Outdated
Comment thread effectful/ops/semantics.py Outdated
Comment thread effectful/ops/syntax.py


class PureInterpretation[T, V](ObjectInterpretation[T, V]):
def __hash__(self):

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.

I wonder if this should be more conservative and use object identity instead of a semantic hash of self.implementations?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the semantic hash is correct and slightly more precise, but it doesn't make much difference in our use.

@jfeser
jfeser requested a review from eb8680 July 24, 2026 20:48
@eb8680
eb8680 merged commit 3d20ed6 into master Jul 24, 2026
28 of 29 checks passed
@eb8680
eb8680 deleted the jf-cache-intp branch July 24, 2026 21:05
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