Skip to content

Commit 32426ea

Browse files
committed
Remove varint encoding and address PR feedback
The varint encoding does help compact the binary node array, but adds maybe a bit to much decoding complexity for only a 20-30% size reduction, and most of the size comes from conditions and results.
1 parent e079f4d commit 32426ea

File tree

14 files changed

+132
-277
lines changed

14 files changed

+132
-277
lines changed

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/logic/bdd/Bdd.java

Lines changed: 30 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.io.UncheckedIOException;
1111
import java.io.Writer;
1212
import java.nio.charset.StandardCharsets;
13+
import java.util.Arrays;
1314
import java.util.function.Consumer;
1415
import software.amazon.smithy.rulesengine.logic.ConditionEvaluator;
1516

@@ -36,9 +37,7 @@ public final class Bdd {
3637
*/
3738
public static final int RESULT_OFFSET = 100_000_000;
3839

39-
private final int[] variables;
40-
private final int[] highs;
41-
private final int[] lows;
40+
private final int[] nodes; // Flat array: [var0, high0, low0, var1, high1, low1, ...]
4241
private final int rootRef;
4342
private final int conditionCount;
4443
private final int resultCount;
@@ -64,27 +63,29 @@ public Bdd(int rootRef, int conditionCount, int resultCount, int nodeCount, Cons
6463

6564
InputNodeConsumer consumer = new InputNodeConsumer(nodeCount);
6665
nodeHandler.accept(consumer);
67-
this.variables = consumer.variables;
68-
this.highs = consumer.highs;
69-
this.lows = consumer.lows;
66+
this.nodes = consumer.nodes;
7067

71-
if (consumer.index != nodeCount) {
72-
throw new IllegalStateException("Expected " + nodeCount + " nodes, but got " + consumer.index);
68+
if (consumer.index != nodeCount * 3) {
69+
throw new IllegalStateException("Expected " + nodeCount + " nodes, but got " + (consumer.index / 3));
7370
}
7471
}
7572

76-
Bdd(int[] variables, int[] highs, int[] lows, int nodeCount, int rootRef, int conditionCount, int resultCount) {
77-
validateArrays(variables, highs, lows, nodeCount);
73+
/**
74+
* Package-private constructor for direct array initialization (used by BddTrait).
75+
*/
76+
Bdd(int rootRef, int conditionCount, int resultCount, int nodeCount, int[] nodes) {
7877
validateCounts(conditionCount, resultCount, nodeCount);
7978
validateRootReference(rootRef, nodeCount);
8079

81-
this.variables = variables;
82-
this.highs = highs;
83-
this.lows = lows;
80+
if (nodes.length != nodeCount * 3) {
81+
throw new IllegalArgumentException("Nodes array length must be nodeCount * 3");
82+
}
83+
8484
this.rootRef = rootRef;
8585
this.conditionCount = conditionCount;
8686
this.resultCount = resultCount;
8787
this.nodeCount = nodeCount;
88+
this.nodes = nodes;
8889
}
8990

9091
private static void validateCounts(int conditionCount, int resultCount, int nodeCount) {
@@ -109,34 +110,19 @@ private static void validateRootReference(int rootRef, int nodeCount) {
109110
}
110111
}
111112

112-
private static void validateArrays(int[] variables, int[] highs, int[] lows, int nodeCount) {
113-
if (variables.length != highs.length || variables.length != lows.length) {
114-
throw new IllegalArgumentException("Array lengths must match: variables=" + variables.length +
115-
", highs=" + highs.length + ", lows=" + lows.length);
116-
} else if (nodeCount > variables.length) {
117-
throw new IllegalArgumentException("Node count (" + nodeCount +
118-
") exceeds array capacity (" + variables.length + ")");
119-
}
120-
}
121-
122113
private static final class InputNodeConsumer implements BddNodeConsumer {
123114
private int index = 0;
124-
private final int[] variables;
125-
private final int[] highs;
126-
private final int[] lows;
115+
private final int[] nodes;
127116

128117
private InputNodeConsumer(int nodeCount) {
129-
this.variables = new int[nodeCount];
130-
this.highs = new int[nodeCount];
131-
this.lows = new int[nodeCount];
118+
this.nodes = new int[nodeCount * 3];
132119
}
133120

134121
@Override
135122
public void accept(int var, int high, int low) {
136-
variables[index] = var;
137-
highs[index] = high;
138-
lows[index] = low;
139-
index++;
123+
nodes[index++] = var;
124+
nodes[index++] = high;
125+
nodes[index++] = low;
140126
}
141127
}
142128

@@ -184,7 +170,7 @@ public int getRootRef() {
184170
*/
185171
public int getVariable(int nodeIndex) {
186172
validateRange(nodeIndex);
187-
return variables[nodeIndex];
173+
return nodes[nodeIndex * 3];
188174
}
189175

190176
private void validateRange(int index) {
@@ -201,7 +187,7 @@ private void validateRange(int index) {
201187
*/
202188
public int getHigh(int nodeIndex) {
203189
validateRange(nodeIndex);
204-
return highs[nodeIndex];
190+
return nodes[nodeIndex * 3 + 1];
205191
}
206192

207193
/**
@@ -212,7 +198,7 @@ public int getHigh(int nodeIndex) {
212198
*/
213199
public int getLow(int nodeIndex) {
214200
validateRange(nodeIndex);
215-
return lows[nodeIndex];
201+
return nodes[nodeIndex * 3 + 2];
216202
}
217203

218204
/**
@@ -222,7 +208,8 @@ public int getLow(int nodeIndex) {
222208
*/
223209
public void getNodes(BddNodeConsumer consumer) {
224210
for (int i = 0; i < nodeCount; i++) {
225-
consumer.accept(variables[i], highs[i], lows[i]);
211+
int base = i * 3;
212+
consumer.accept(nodes[base], nodes[base + 1], nodes[base + 2]);
226213
}
227214
}
228215

@@ -234,14 +221,13 @@ public void getNodes(BddNodeConsumer consumer) {
234221
*/
235222
public int evaluate(ConditionEvaluator ev) {
236223
int ref = rootRef;
237-
int[] vars = this.variables;
238-
int[] hi = this.highs;
239-
int[] lo = this.lows;
224+
int[] n = this.nodes;
240225

241226
while (isNodeReference(ref)) {
242-
int idx = ref > 0 ? ref - 1 : -ref - 1; // Math.abs
227+
int idx = ref > 0 ? ref - 1 : -ref - 1;
228+
int base = idx * 3;
243229
// test ^ complement, pick hi or lo
244-
ref = (ev.test(vars[idx]) ^ (ref < 0)) ? hi[idx] : lo[idx];
230+
ref = (ev.test(n[base]) ^ (ref < 0)) ? n[base + 1] : n[base + 2];
245231
}
246232

247233
return isTerminal(ref) ? -1 : ref - RESULT_OFFSET;
@@ -303,27 +289,12 @@ public boolean equals(Object obj) {
303289
return false;
304290
}
305291

306-
// Now check the views of arrays of each.
307-
for (int i = 0; i < nodeCount; i++) {
308-
if (variables[i] != other.variables[i] || highs[i] != other.highs[i] || lows[i] != other.lows[i]) {
309-
return false;
310-
}
311-
}
312-
313-
return true;
292+
return Arrays.equals(nodes, other.nodes);
314293
}
315294

316295
@Override
317296
public int hashCode() {
318-
int hash = 31 * rootRef + nodeCount;
319-
// Sample up to 16 nodes distributed across the BDD
320-
int step = Math.max(1, nodeCount / 16);
321-
for (int i = 0; i < nodeCount; i += step) {
322-
hash = 31 * hash + variables[i];
323-
hash = 31 * hash + highs[i];
324-
hash = 31 * hash + lows[i];
325-
}
326-
return hash;
297+
return 31 * rootRef + nodeCount + Arrays.hashCode(nodes);
327298
}
328299

329300
@Override

0 commit comments

Comments
 (0)