Skip to content

Commit 54f9bbc

Browse files
authored
Optimize NonDuplicateEqApp-features (#3829)
2 parents f1eb1c1 + 70eacaf commit 54f9bbc

3 files changed

Lines changed: 233 additions & 22 deletions

File tree

key.core/src/main/java/de/uka/ilkd/key/strategy/feature/AbstractNonDuplicateAppFeature.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,17 @@ protected boolean noDuplicateFindTaclet(TacletApp app,
125125
final Node node = goal.node();
126126
final AppliedRuleAppsNameCache cache =
127127
node.proof().getServices().getCaches().getAppliedRuleAppsNameCache();
128-
List<RuleApp> apps = cache.get(node, app.rule().name());
129-
130-
// Check all rules with this name
128+
// A duplicate must agree on the focus term up to term labels (every comparePio variant,
129+
// including the modulo-position one, implies this), so it shares the candidate's focus-term
130+
// fingerprint and can only be in that bucket. A find-less application (pos == null) lands
131+
// in
132+
// bucket 0, where all find-less applications of this name live (a taclet name is either
133+
// find
134+
// or find-less, never both).
135+
final int fingerprint = pos == null ? 0 : pos.subTerm().hashCode();
136+
List<RuleApp> apps = cache.get(node, app.rule().name(), fingerprint);
137+
138+
// Check all rules with this name in the same fingerprint bucket
131139
for (RuleApp a : apps) {
132140
if (sameApplication(a, app, pos)) {
133141
return false;

key.core/src/main/java/de/uka/ilkd/key/strategy/feature/AppliedRuleAppsNameCache.java

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,33 @@
1111

1212
import org.key_project.logic.Name;
1313
import org.key_project.prover.rules.RuleApp;
14+
import org.key_project.prover.sequent.PosInOccurrence;
1415
import org.key_project.util.LRUCache;
1516

1617
import org.jspecify.annotations.NonNull;
1718

1819
/**
1920
* Establishes a cache for the applied rule apps to query them by name.
2021
* See the get method for additional required constraints for correctness.
22+
* <p>
23+
* Within a rule name the applied apps are additionally bucketed by an application
24+
* <em>fingerprint</em>: the hash code of the application's focus term ({@code pos.subTerm()}), or
25+
* {@code 0} for find-less apps. This turns the duplicate search in
26+
* {@link AbstractNonDuplicateAppFeature} from a linear scan over all same-named applications on the
27+
* branch into an O(1) bucket lookup. It is sound for every duplicate check because each one's
28+
* {@code comparePio} implies the two focus terms are equal up to term labels:
29+
* {@link NonDuplicateAppFeature} (equal positions), {@link EqNonDuplicateAppFeature} (equal
30+
* positions modulo formula renaming) and {@link NonDuplicateAppModPositionFeature} (equal focus
31+
* terms modulo irrelevant labels). As {@code TermImpl.hashCode} ignores term labels, focus terms
32+
* that are equal up to labels always share a fingerprint, so a duplicate can only ever live in the
33+
* candidate's own bucket -- including for the modulo-position variant, which deliberately matches
34+
* the same focus term at different sequent positions.
2135
*
2236
* @author Julian Wiesler
2337
*/
2438
public class AppliedRuleAppsNameCache {
25-
/** cache of all applied rules by name of a node */
26-
private final LRUCache<Node, HashMap<Name, List<RuleApp>>> cache =
39+
/** cache of all applied rules of a node, by rule name and then by application fingerprint */
40+
private final LRUCache<Node, HashMap<Name, HashMap<Integer, List<RuleApp>>>> cache =
2741
new LRUCache<>(32);
2842

2943
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@@ -32,21 +46,40 @@ public class AppliedRuleAppsNameCache {
3246

3347
public AppliedRuleAppsNameCache() {}
3448

49+
/**
50+
* @return the fingerprint of a rule application: the hash code of its focus term, or {@code 0}
51+
* for a find-less application. The hash ignores term labels (as does
52+
* {@code TermImpl.hashCode}); see the class comment for why this is sound for all
53+
* duplicate checks. A find-less app uses {@code 0}; should a focus term also hash to
54+
* {@code 0} the only effect is a shared bucket, which the {@code sameApplication} scan
55+
* resolves.
56+
*/
57+
private static int fingerprint(RuleApp app) {
58+
final PosInOccurrence pio = app.posInOccurrence();
59+
return pio == null ? 0 : pio.subTerm().hashCode();
60+
}
61+
62+
private static void add(HashMap<Name, HashMap<Integer, List<RuleApp>>> nodeCache, RuleApp app) {
63+
nodeCache.computeIfAbsent(app.rule().name(), k -> new HashMap<>())
64+
.computeIfAbsent(fingerprint(app), k -> new ArrayList<>())
65+
.add(app);
66+
}
67+
3568
/**
3669
* Fills the cache value of this instance for node
3770
*
3871
* @param node the node
3972
* @return the value
4073
*/
41-
private @NonNull HashMap<Name, List<RuleApp>> fillCacheForNode(
74+
private @NonNull HashMap<Name, HashMap<Integer, List<RuleApp>>> fillCacheForNode(
4275
Node node) {
43-
HashMap<Name, List<RuleApp>> nodeCache;
76+
HashMap<Name, HashMap<Integer, List<RuleApp>>> nodeCache;
4477
try {
4578
writeLock.lock();
4679
nodeCache = cache.get(node);
4780
if (nodeCache == null) {
4881
// Try to use parent cache to initialize the new cache
49-
HashMap<Name, List<RuleApp>> parentCache =
82+
HashMap<Name, HashMap<Integer, List<RuleApp>>> parentCache =
5083
node.root() ? null : cache.get(node.parent());
5184
nodeCache = new HashMap<>();
5285

@@ -56,19 +89,21 @@ public AppliedRuleAppsNameCache() {}
5689
nodeCache = parentCache;
5790
} else {
5891
// Copy the parent cache
59-
for (Map.Entry<Name, List<RuleApp>> entry : parentCache
92+
for (Map.Entry<Name, HashMap<Integer, List<RuleApp>>> entry : parentCache
6093
.entrySet()) {
61-
nodeCache.put(entry.getKey(), new ArrayList<>(entry.getValue()));
94+
HashMap<Integer, List<RuleApp>> buckets = new HashMap<>();
95+
for (Map.Entry<Integer, List<RuleApp>> bucket : entry.getValue()
96+
.entrySet()) {
97+
buckets.put(bucket.getKey(), new ArrayList<>(bucket.getValue()));
98+
}
99+
nodeCache.put(entry.getKey(), buckets);
62100
}
63101
}
64102

65103
// Parent did not have a rule applied when we calculated this, add the rule
66104
// applied
67105
// there
68-
RuleApp parentApp =
69-
node.parent().getAppliedRuleApp();
70-
nodeCache.computeIfAbsent(parentApp.rule().name(), k -> new ArrayList<>())
71-
.add(parentApp);
106+
add(nodeCache, node.parent().getAppliedRuleApp());
72107

73108
// If this is an inner node, we hope we will never revisit it, remove it from
74109
// the
@@ -81,10 +116,7 @@ public AppliedRuleAppsNameCache() {}
81116
Node current = node;
82117
while (!current.root()) {
83118
final Node par = current.parent();
84-
85-
RuleApp a = par.getAppliedRuleApp();
86-
nodeCache.computeIfAbsent(a.rule().name(), k -> new ArrayList<>()).add(a);
87-
119+
add(nodeCache, par.getAppliedRuleApp());
88120
current = par;
89121
}
90122
}
@@ -99,7 +131,8 @@ public AppliedRuleAppsNameCache() {}
99131
}
100132

101133
/**
102-
* Gets rule apps applied to any node before the given node with the given name.
134+
* Gets rule apps applied to any node before the given node with the given name and the given
135+
* application fingerprint (see {@link #fingerprint(RuleApp)}).
103136
* <p>
104137
* Multiple assumptions about nodes:
105138
* * The given node is a leaf, no children, no applied rule
@@ -109,15 +142,16 @@ public AppliedRuleAppsNameCache() {}
109142
*
110143
* @param node the node
111144
* @param name the name
145+
* @param fingerprint the application fingerprint to restrict to
112146
* @return rule apps
113147
*/
114148
public @NonNull List<RuleApp> get(@NonNull Node node,
115-
@NonNull Name name) {
149+
@NonNull Name name, int fingerprint) {
116150
if (node.getAppliedRuleApp() != null || node.childrenCount() != 0) {
117151
throw new AssertionFailure("Expected an empty leaf node");
118152
}
119153

120-
HashMap<Name, List<RuleApp>> nodeCache;
154+
HashMap<Name, HashMap<Integer, List<RuleApp>>> nodeCache;
121155
try {
122156
readLock.lock();
123157
nodeCache = cache.get(node);
@@ -129,7 +163,11 @@ public AppliedRuleAppsNameCache() {}
129163
nodeCache = fillCacheForNode(node);
130164
}
131165

132-
List<RuleApp> apps = nodeCache.get(name);
166+
HashMap<Integer, List<RuleApp>> byFingerprint = nodeCache.get(name);
167+
if (byFingerprint == null) {
168+
return Collections.emptyList();
169+
}
170+
List<RuleApp> apps = byFingerprint.get(fingerprint);
133171
return apps == null ? Collections.emptyList() : Collections.unmodifiableList(apps);
134172
}
135173
}
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/* This file is part of KeY - https://key-project.org
2+
* KeY is licensed under the GNU General Public License Version 2
3+
* SPDX-License-Identifier: GPL-2.0-only */
4+
package de.uka.ilkd.key.strategy.feature;
5+
6+
import de.uka.ilkd.key.proof.BuiltInRuleAppIndex;
7+
import de.uka.ilkd.key.proof.BuiltInRuleIndex;
8+
import de.uka.ilkd.key.proof.Goal;
9+
import de.uka.ilkd.key.proof.Node;
10+
import de.uka.ilkd.key.proof.Proof;
11+
import de.uka.ilkd.key.proof.TacletIndex;
12+
import de.uka.ilkd.key.proof.TacletIndexKit;
13+
import de.uka.ilkd.key.proof.calculus.JavaDLSequentKit;
14+
import de.uka.ilkd.key.rule.TacletApp;
15+
import de.uka.ilkd.key.rule.TacletForTests;
16+
17+
import org.key_project.logic.PosInTerm;
18+
import org.key_project.prover.proof.rulefilter.TacletFilter;
19+
import org.key_project.prover.sequent.PosInOccurrence;
20+
import org.key_project.prover.sequent.Sequent;
21+
import org.key_project.prover.sequent.SequentFormula;
22+
import org.key_project.prover.strategy.costbased.MutableState;
23+
import org.key_project.prover.strategy.costbased.RuleAppCost;
24+
import org.key_project.prover.strategy.costbased.TopRuleAppCost;
25+
import org.key_project.prover.strategy.costbased.feature.Feature;
26+
import org.key_project.util.collection.ImmutableList;
27+
import org.key_project.util.collection.ImmutableSLList;
28+
29+
import org.junit.jupiter.api.BeforeAll;
30+
import org.junit.jupiter.api.Test;
31+
32+
import static org.junit.jupiter.api.Assertions.assertNotSame;
33+
import static org.junit.jupiter.api.Assertions.assertSame;
34+
35+
/**
36+
* Tests duplicate detection of {@link NonDuplicateAppFeature} and {@link EqNonDuplicateAppFeature},
37+
* which go through the fingerprint-bucketed {@link AppliedRuleAppsNameCache}. The two key
38+
* properties: an application already performed on the branch is rejected (cost
39+
* {@link TopRuleAppCost}), and the fingerprint bucketing never hides a real duplicate nor invents
40+
* one for the same taclet applied at a <em>different</em> position (a different bucket).
41+
*/
42+
public class TestNonDuplicateAppFeature {
43+
44+
@BeforeAll
45+
public static void setUp() {
46+
TacletForTests.parse();
47+
}
48+
49+
/** Root sequent {@code ==> A -> B, B -> A} (two distinct succedent formulas). */
50+
private static Sequent twoImplications() {
51+
ImmutableList<SequentFormula> succ = ImmutableSLList.<SequentFormula>nil()
52+
.append(new SequentFormula(TacletForTests.parseTerm("A -> B")))
53+
.append(new SequentFormula(TacletForTests.parseTerm("B -> A")));
54+
return JavaDLSequentKit.createSequent(ImmutableSLList.nil(), succ);
55+
}
56+
57+
private static Goal goalFor(Node n, TacletIndex idx) {
58+
return new Goal(n, idx, new BuiltInRuleAppIndex(new BuiltInRuleIndex()),
59+
n.proof().getServices());
60+
}
61+
62+
private static PosInOccurrence topSucc(Node n, int idx) {
63+
return new PosInOccurrence(n.sequent().succedent().get(idx), PosInTerm.getTopLevel(),
64+
false);
65+
}
66+
67+
private static TacletApp appAt(Goal goal, PosInOccurrence pos) {
68+
return goal.ruleAppIndex().getTacletAppAt(TacletFilter.TRUE, pos, null).head();
69+
}
70+
71+
/**
72+
* Build {@code root --(imp_right at succedent #applyIdx)--> child(leaf)} and return the feature
73+
* cost of applying imp_right at succedent #candIdx on the child leaf.
74+
*/
75+
private static RuleAppCost cost(Feature feature, int applyIdx, int candIdx) {
76+
Proof proof = new Proof("TestNonDuplicateAppFeature", TacletForTests.initConfig());
77+
Node root = new Node(proof, twoImplications());
78+
proof.setRoot(root);
79+
80+
TacletIndex idx = TacletIndexKit.getKit().createTacletIndex();
81+
idx.add(TacletForTests.lookupTaclet("imp_right"));
82+
Goal rootGoal = goalFor(root, idx);
83+
84+
final PosInOccurrence applyPos = topSucc(root, applyIdx);
85+
final PosInOccurrence candPos = topSucc(root, candIdx);
86+
final TacletApp applied = appAt(rootGoal, applyPos);
87+
final TacletApp candidate = candIdx == applyIdx ? applied : appAt(rootGoal, candPos);
88+
89+
root.setAppliedRuleApp(applied);
90+
Node child = new Node(proof, root.sequent(), root);
91+
root.add(child);
92+
93+
return feature.computeCost(candidate, candPos, goalFor(child, idx), new MutableState());
94+
}
95+
96+
@Test
97+
public void nonDuplicateRejectsAlreadyAppliedApp() {
98+
assertSame(TopRuleAppCost.INSTANCE, cost(NonDuplicateAppFeature.INSTANCE, 0, 0),
99+
"re-applying the same taclet at the same position must be rejected as a duplicate");
100+
}
101+
102+
@Test
103+
public void nonDuplicateAcceptsAppAtDifferentPosition() {
104+
assertNotSame(TopRuleAppCost.INSTANCE, cost(NonDuplicateAppFeature.INSTANCE, 1, 0),
105+
"the same taclet at a different position is not a duplicate (different fingerprint bucket)");
106+
}
107+
108+
@Test
109+
public void eqNonDuplicateRejectsAlreadyAppliedApp() {
110+
assertSame(TopRuleAppCost.INSTANCE, cost(EqNonDuplicateAppFeature.INSTANCE, 0, 0),
111+
"eqEquals variant must also reject an already-applied application");
112+
}
113+
114+
@Test
115+
public void eqNonDuplicateAcceptsAppAtDifferentPosition() {
116+
assertNotSame(TopRuleAppCost.INSTANCE, cost(EqNonDuplicateAppFeature.INSTANCE, 1, 0),
117+
"eqEquals variant must accept the same taclet at a different position");
118+
}
119+
120+
/**
121+
* Apply a rewrite (TestApplyTaclet_contradiction, {@code find(b->c)}) at one occurrence of
122+
* {@code A -> B} in {@code ==> (A -> B) & (A -> B)}, then return the cost of applying it at the
123+
* <em>other</em> occurrence: same focus term, different sequent position. This is the case the
124+
* fingerprint must not get wrong -- the modulo-position variant treats it as a duplicate, the
125+
* plain variant does not.
126+
*/
127+
private static RuleAppCost crossPositionCost(Feature feature) {
128+
Proof proof = new Proof("TestNonDuplicateAppFeature", TacletForTests.initConfig());
129+
SequentFormula conj = new SequentFormula(TacletForTests.parseTerm("(A -> B) & (A -> B)"));
130+
Node root = new Node(proof,
131+
JavaDLSequentKit.createSequent(ImmutableSLList.nil(), ImmutableSLList.singleton(conj)));
132+
proof.setRoot(root);
133+
134+
TacletIndex idx = TacletIndexKit.getKit().createTacletIndex();
135+
idx.add(TacletForTests.lookupTaclet("TestApplyTaclet_contradiction"));
136+
Goal rootGoal = goalFor(root, idx);
137+
138+
final SequentFormula f = root.sequent().succedent().get(0);
139+
final PosInOccurrence leftConjunct =
140+
new PosInOccurrence(f, PosInTerm.getTopLevel().down(0), false);
141+
final PosInOccurrence rightConjunct =
142+
new PosInOccurrence(f, PosInTerm.getTopLevel().down(1), false);
143+
144+
root.setAppliedRuleApp(appAt(rootGoal, leftConjunct));
145+
Node child = new Node(proof, root.sequent(), root);
146+
root.add(child);
147+
148+
return feature.computeCost(appAt(rootGoal, rightConjunct), rightConjunct,
149+
goalFor(child, idx),
150+
new MutableState());
151+
}
152+
153+
@Test
154+
public void modPositionRejectsSameFocusAtDifferentPosition() {
155+
assertSame(TopRuleAppCost.INSTANCE,
156+
crossPositionCost(NonDuplicateAppModPositionFeature.INSTANCE),
157+
"modulo-position variant must treat the same focus term at a different position as a duplicate");
158+
}
159+
160+
@Test
161+
public void nonDuplicateAllowsSameFocusAtDifferentPosition() {
162+
assertNotSame(TopRuleAppCost.INSTANCE, crossPositionCost(NonDuplicateAppFeature.INSTANCE),
163+
"plain variant must not treat a different position as a duplicate even with equal focus term");
164+
}
165+
}

0 commit comments

Comments
 (0)