Performance (5/5): operator-indexed parking of assumes-incomplete rule-app bases#3838
Draft
unp1 wants to merge 2 commits into
Draft
Performance (5/5): operator-indexed parking of assumes-incomplete rule-app bases#3838unp1 wants to merge 2 commits into
unp1 wants to merge 2 commits into
Conversation
Park assumes-incomplete taclet bases that re-expand to nothing (97-99.6% of base re-expansions) out of the active queue, and wake them by a precise operator index: index each parked base by the concrete top operator(s) of its \assumes formulas (resolved through the find-match's SV instantiations); wake exactly the bases whose operator matches a formula added/modified that round (Goal.fireSequentChanged -> sequentChanged), walking the changed formula's update-prefix spine (a sound superset, since the assumes matcher strips the update context). Only effectively-indexable bases are parked; unbound-generic tops stay in the queue. Insertion-ordered (LinkedHashMap/Set) for determinism; clone() deep-copies the index. Active by default. Provability-safe on the full real RAP suite (681 goals) once the redundant order-fragile lenOfSeqSubEQ is dropped from automode (see follow-up commit). Not byte-identical (reordering shifts proof shapes, all still close); ~40% faster automode on the perfTest goals.
lenOfSeqSubEQ rewrites seqLen(EQ) via an antecedent equation EQ = seqSub(...). It is redundant for completeness -- the direct lenOfSeqSub suffices (full RAP closes all 681 goals without it). It is also order-fragile: when the negated-goal equation seqSub(s,0,i)=seqSub(s,0,i+5) is reused as an \assumes to simplify, the simplification reproduces that same formula, and the duplicate/cycle guard then refuses to re-apply subSeqEqual -> the goal dead-ends. The original rule order avoided this by luck; any reordering (e.g. assumes-parking) can expose it. Only the \heuristics is commented out, so the taclet stays defined and existing proofs that applied it still load/replay.
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
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.
This PR is 5/5 of a performance series that prepares a clearer match → evaluate → apply pipeline before larger optimisations; it builds on the compiled matcher (1/5, #3831).
📖 Developer docs: Performance Optimizations (3.1)
Intended Change
Profiling the rule-app queue shows the dominant remaining churn comes from assumes-incomplete taclet bases that are re-popped and re-expanded every single round only to produce nothing: 96.8% of queue pops fail at an unmatched
\assumes, and 97–99.6% of the resulting re-expansions yield no new instance. This PR stops re-expanding such bases until a formula they could actually match appears — an operator-indexed parking scheme.Mechanism:
\assumesformulas are effectively indexable (every assumes top operator is concrete, or a schema variable already bound by the find-match) is parked out of the active queue, indexed by those concrete top operator(s).Because a woken base re-expands on the identical round with the identical age and cost, the result is byte-identical for the parking mechanism itself. All index structures are insertion-ordered (
LinkedHashMap/LinkedHashSet) so no non-determinism is introduced.This PR also drops the order-fragile
lenOfSeqSubEQheuristic from automode (the taclet stays defined, only its\heuristicsis commented out). That taclet is redundant for completeness (the directlenOfSeqSubsuffices — full RAP closes all goals without it) but is order-fragile: it can dead-end a goal via a duplicate/cycle guard when its own result reproduces the equation it started from. The original rule order avoided this by luck; any reordering (including parking) can expose it. Removing it makes automode robust to rule-order changes.flowchart LR A["assumes-incomplete base"] --> B{"indexable?"} B -- no --> C["re-expand each round"] B -- yes --> D["park; wake on matching op"]Type of pull request
lenOfSeqSubEQheuristic)Performance
Measured on the 6-problem
perfTestset (median of 3 runs). Parking is not byte-identical (it can shift proof shapes — all still close), so the table reports both node counts and automode time.Node counts shift slightly (parking is not byte-identical — all proofs still close), confirming it is a queue-scheduling change rather than a different search.
Parking targets the wasted re-expansions of assumes-incomplete bases. Across the six problems:
That is ~4.4× fewer; the remaining re-expansions are mostly non-indexable bases (unbound-generic
\assumestops) that parking deliberately leaves in the queue.Ensuring quality
lenOfSeqSubEQis dropped.-Dkey.queue.parkAssumesgate is removed).📖 Conceptual overview in the developer docs: Performance Optimizations (3.1)
Additional information and contact(s)
This PR has been done with AI tooling support.
The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.