Skip to content

Commit d054f14

Browse files
committed
Improve initial ordering and add coalesce
Our previous initial ordering could result in pathalogical orderings if it decided to moving something very early from the CFG to very late. This is in fact what happened when I added a coalesce method: it moved an early discriminating condition to very late, which blew up the BDD from ~40K nodes to 5.1M. This taught me that we really shouldn't throw away the ordering found in the CFG, and instead should leverage it when determining the initial ordering since it inherently gates logic and keeps related conditions together. So now the initial ordering is based on the CFG ordering and also on cone analysis (basically how many downstream nodes a node affects). We now get an initial ordering ~3K nodes, and with the coalesce method, we can now sift S3 down to ~800 nodes instead of ~1000. The coalesce function is added here so that we can fold bind-then-test conditions into a single condition. The current endpoints type system has strict nullability requirements. So you can't do a substring test and pass that directly into something that expects a non-null value. You have to first do the nullable function, then assign that to a value, then the next condition is inherently guarded and only called if the assigned value is non-null (the assignment acts as an implicit guard). The coalsce function allows us to identify these patterns and inline the test into a single condition by defaulting null to the zero value of the return type (integer=0, string="", array=[]). We only coalesce when the comparison is not to literally the zero value. When coalesce was added, it uncovered the original brittle ordering, leading to the much improved ordering in this PR.
1 parent 23c84f5 commit d054f14

File tree

58 files changed

+2870
-1067
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+2870
-1067
lines changed

smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/AwsPartition.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,6 @@ public Value evaluate(List<Value> arguments) {
184184
public AwsPartition createFunction(FunctionNode functionNode) {
185185
return new AwsPartition(functionNode);
186186
}
187-
188-
@Override
189-
public int getCostHeuristic() {
190-
return 6;
191-
}
192187
}
193188

194189
/**

smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/IsVirtualHostableS3Bucket.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,6 @@ public Value evaluate(List<Value> arguments) {
9797
public IsVirtualHostableS3Bucket createFunction(FunctionNode functionNode) {
9898
return new IsVirtualHostableS3Bucket(functionNode);
9999
}
100-
101-
@Override
102-
public int getCostHeuristic() {
103-
return 8;
104-
}
105100
}
106101

107102
/**

smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/ParseArn.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,5 @@ public Value evaluate(List<Value> arguments) {
125125
public ParseArn createFunction(FunctionNode functionNode) {
126126
return new ParseArn(functionNode);
127127
}
128-
129-
@Override
130-
public int getCostHeuristic() {
131-
return 9;
132-
}
133128
}
134129
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/ArrayType.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44
*/
55
package software.amazon.smithy.rulesengine.language.evaluation.type;
66

7+
import java.util.Collections;
78
import java.util.Objects;
9+
import java.util.Optional;
10+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
811

912
/**
1013
* The "array" type, which contains entries of a member type.
1114
*/
1215
public final class ArrayType extends AbstractType {
16+
17+
private static final Optional<Literal> ZERO = Optional.of(Literal.tupleLiteral(Collections.emptyList()));
1318
private final Type member;
1419

1520
ArrayType(Type member) {
@@ -51,4 +56,9 @@ public int hashCode() {
5156
public String toString() {
5257
return String.format("ArrayType[%s]", member);
5358
}
59+
60+
@Override
61+
public Optional<Literal> getZeroValue() {
62+
return ZERO;
63+
}
5464
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/BooleanType.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@
44
*/
55
package software.amazon.smithy.rulesengine.language.evaluation.type;
66

7+
import java.util.Optional;
8+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
9+
710
/**
811
* The "boolean" type.
912
*/
1013
public final class BooleanType extends AbstractType {
1114

15+
private static final Optional<Literal> ZERO = Optional.of(Literal.of(false));
1216
static final BooleanType INSTANCE = new BooleanType();
1317

1418
BooleanType() {}
@@ -17,4 +21,9 @@ public final class BooleanType extends AbstractType {
1721
public BooleanType expectBooleanType() {
1822
return this;
1923
}
24+
25+
@Override
26+
public Optional<Literal> getZeroValue() {
27+
return ZERO;
28+
}
2029
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/IntegerType.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@
44
*/
55
package software.amazon.smithy.rulesengine.language.evaluation.type;
66

7+
import java.util.Optional;
8+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
9+
710
/**
811
* The "integer" type.
912
*/
1013
public final class IntegerType extends AbstractType {
1114

15+
private static final Optional<Literal> ZERO = Optional.of(Literal.of(0));
1216
static final IntegerType INSTANCE = new IntegerType();
1317

1418
IntegerType() {}
@@ -17,4 +21,9 @@ public final class IntegerType extends AbstractType {
1721
public IntegerType expectIntegerType() {
1822
return this;
1923
}
24+
25+
@Override
26+
public Optional<Literal> getZeroValue() {
27+
return ZERO;
28+
}
2029
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/OptionalType.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
package software.amazon.smithy.rulesengine.language.evaluation.type;
66

77
import java.util.Objects;
8+
import java.util.Optional;
89
import software.amazon.smithy.rulesengine.language.error.InnerParseError;
10+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
911

1012
/**
1113
* The "optional" type, a container for a type that may or may not be present.
@@ -78,4 +80,9 @@ public int hashCode() {
7880
public String toString() {
7981
return String.format("OptionalType[%s]", inner);
8082
}
83+
84+
@Override
85+
public Optional<Literal> getZeroValue() {
86+
return inner.getZeroValue();
87+
}
8188
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/StringType.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@
44
*/
55
package software.amazon.smithy.rulesengine.language.evaluation.type;
66

7+
import java.util.Optional;
8+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
9+
710
/**
811
* The "string" type.
912
*/
1013
public final class StringType extends AbstractType {
1114

15+
private static final Optional<Literal> ZERO = Optional.of(Literal.of(""));
1216
static final StringType INSTANCE = new StringType();
1317

1418
StringType() {}
@@ -17,4 +21,9 @@ public final class StringType extends AbstractType {
1721
public StringType expectStringType() {
1822
return this;
1923
}
24+
25+
@Override
26+
public Optional<Literal> getZeroValue() {
27+
return ZERO;
28+
}
2029
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import java.util.List;
88
import java.util.Map;
9+
import java.util.Optional;
910
import software.amazon.smithy.rulesengine.language.error.InnerParseError;
1011
import software.amazon.smithy.rulesengine.language.syntax.Identifier;
12+
import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal;
1113
import software.amazon.smithy.rulesengine.language.syntax.parameters.ParameterType;
1214
import software.amazon.smithy.utils.SmithyUnstableApi;
1315

@@ -129,4 +131,17 @@ default StringType expectStringType() throws InnerParseError {
129131
default TupleType expectTupleType() throws InnerParseError {
130132
throw new InnerParseError("Expected tuple but found " + this);
131133
}
134+
135+
/**
136+
* Gets the default zero-value of the type as a Literal.
137+
*
138+
* <p>Strings, booleans, integers, and arrays have zero values. Other types do not. E.g., a map might have
139+
* required properties, and the behavior of a tuple _seems_ to imply that each member is required. Optionals
140+
* return the zero value of its inner type.
141+
*
142+
* @return the default zero value.
143+
*/
144+
default Optional<Literal> getZeroValue() {
145+
return Optional.empty();
146+
}
132147
}

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/BooleanValue.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,17 @@
1414
* A boolean value of true or false.
1515
*/
1616
public final class BooleanValue extends Value {
17+
18+
static final BooleanValue TRUE = new BooleanValue(true);
19+
static final BooleanValue FALSE = new BooleanValue(false);
20+
1721
private final boolean value;
1822

19-
BooleanValue(boolean value) {
23+
static BooleanValue create(boolean v) {
24+
return v ? TRUE : FALSE;
25+
}
26+
27+
private BooleanValue(boolean value) {
2028
super(SourceLocation.none());
2129
this.value = value;
2230
}

0 commit comments

Comments
 (0)