Let a term be evaluated again, and look up a signature once per combinator - #100
Merged
Conversation
![tudo-seal-workflows[bot]](https://avatars.githubusercontent.com/in/2312552?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Benchmark CoSy
Details
| Benchmark suite | Current: e496e28 | Previous: be71b5d | Ratio |
|---|---|---|---|
benchmarks/test_benchmark_maximal_elements.py::test_benchmark_maximal_elements |
9.749118761312753 iter/sec (stddev: 0.000273129446901472) |
9.608281351562356 iter/sec (stddev: 0.010017085420616013) |
0.99 |
benchmarks/test_benchmark_maze.py::test_benchmark_maze |
3.743497299056593 iter/sec (stddev: 0.02100845057698785) |
3.9235166334572096 iter/sec (stddev: 0.018797551923491352) |
1.05 |
benchmarks/test_benchmark_maze_contains.py::test_benchmark_maze_contains |
3.426409647594738 iter/sec (stddev: 0.01924185941580888) |
3.5391214334611916 iter/sec (stddev: 0.025877608823890046) |
1.03 |
benchmarks/test_benchmark_maze_loopfree.py::test_benchmark_maze_loopfree |
3.648644110031215 iter/sec (stddev: 0.020082525573004724) |
3.8477221717914887 iter/sec (stddev: 0.02072136799063595) |
1.05 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #100 +/- ##
===========================================
+ Coverage 79.28% 79.99% +0.70%
===========================================
Files 47 47
Lines 3833 3944 +111
Branches 532 536 +4
===========================================
+ Hits 3039 3155 +116
+ Misses 693 689 -4
+ Partials 101 100 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…nator
`Tree.interpret` kept the result of its last evaluation on the node, keyed on
`id(interpretation)`, and answered from it. That is not a faster evaluation. It
is a different one. An interpretation may have side effects and may answer
differently on every call. A fitness that averages a noisy measurement is the
ordinary case, and a term that replays its first answer denies it. The attribute
comment said as much: "Breaks for non-deterministic interpretations."
Three ways it went wrong, and none of them was reported anywhere.
* A combinator with a side effect ran once and then never again, however often
the term was evaluated.
* `id(interpretation)` names an address, not contents. A dictionary changed in
place is the same object with a different meaning for every symbol in it,
and the old entry still answered for it.
* The entry sits on a node, and a node is shared by every term built around
it, so a subterm evaluated in one term reported that result through all of
them.
Removing it costs what it was there for, and that cost was real. Evaluating a
term asked `inspect.signature` for the parameters of every combinator
*occurrence*, at about 5.4 microseconds each, so a chain of five thousand nodes
over two combinators paid for it five thousand times to learn two answers.
That is the part worth remembering, and it is remembered on its own. A module
memo maps a combinator to its parameters. It holds metadata about a combinator,
never the result of applying one, so every combinator is still called on every
evaluation and the three cases above stay intact.
Measured against the previous state, same machine, same run, with fresh terms
per repetition: one term of 5000 nodes 29.0 ms to 11.8 ms, and two thousand
terms of twenty nodes over seven combinators 215.3 ms to 80.7 ms. Where the old
cache answered instead of evaluating, the new state is slower and has to be. The
same term evaluated a thousand times took 0.3 ms and now takes 41.5 ms, because
a thousand evaluations now happen. Enumerating three thousand terms under
predicates over interpreted subterms goes from 1.0 ms to 1.2 ms. The four
benchmarks are unchanged.
The memo is bounded at 1024 entries, because an algebra is typically built
inside the call that evaluates the term. Every example here does it that way, so
each evaluation produces a fresh set of callables that is used once and
unreachable afterwards. Measured on that pattern over 50000 evaluations, an
unbounded memo grows to 200000 entries and becomes slower than the bounded one,
1434 ms against 1245 ms, because its table keeps being rebuilt. The bound is
twenty times the largest algebra in use, since under a round trip through a
working set an LRU degrades all at once rather than gradually: over 200
combinators, `maxsize=128` runs about ninety times slower than `maxsize=1024`.
Lowering the bound below the working set and removing it both fail a test.
A combinator that cannot key the memo is inspected directly rather than
rejected. One written as a value object, with `__eq__` and therefore no
`__hash__`, worked before and still does. One without an introspectable
signature is still reported as the `TypeError` it always was, on every
evaluation, and never replaced by a value.
The four tests that pinned the old behavior are gone, including the one that
required a combinator with a side effect not to run a second time.
`tests/test_interpretation_semantics.py` states the opposite promise, so the
next attempt to cache results has something to fail against.
FelixLaarmann
force-pushed
the
bugfix/interpretation-caching
branch
from
August 21, 2026 08:12
ebd9b57 to
3e62e03
Compare
``_parameters_cached`` and ``_parameters_of`` took ``Any`` while their docstrings said callable and ``inspect.signature`` requires one. The annotation now says the same thing as the documentation.
``parameters_of_c`` did not say which combinator it described, and it was declared one line above the ``current_combinator`` it reads from. It is now ``parameters_of_current_combinator`` and follows that declaration.
The comment above ``@lru_cache`` argued from a fresh algebra to the bound without stating the step in between. That step is that the cache key is the combinator object itself, and a function or a lambda hashes by identity, so an algebra rebuilt for every evaluation presents new keys for the same signatures. Without it the paragraph reads as though the key changed on its own. The comment now states it, keeps the two arguments the bound carries apart, and says why the bound is 1024 when a working set is at most 49. Two places still described the per-node interpretation cache this branch removes, one as a reason for sharing nodes across a generation and one in the module docstring of the invariant tests. Both are gone.
mrhaandi
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #98.
The measurements and the case for removing the result cache are in #98 and are not repeated here.
This is what the change looks like, and it answers the question I left open there, namely how long
an entry should live.
Tree._interpretedand the two cache sites ininterpretare gone, so every combinator isapplied on every evaluation again. In their place,
interpretasks_parameters_offor theparameters of a combinator, and that lookup memoizes on the combinator object.
The memo holds metadata about a combinator, never the result of applying one. Parameters follow
from the callable and not from its arguments, so an entry cannot go stale the way a result can.
The assumption behind it is weaker than one
interpretalready made, since the body determinesthe parameters of an occurrence once and keeps them for the whole argument distribution, cache or
no cache. It can be wrong only if a callable changes its signature between evaluations, and that
ends in a
TypeErrornaming the combinator rather than in a wrong value.