Skip to content

Commit 4c97b7c

Browse files
authored
S3 tree optimizations (smithy-lang#2913)
* Improve BDD sifting (2x speed, more reduction) - Add AdaptiveEffort to dynamically adjust optimization parameters based on observed improvement rates (increase effort when making progress, decrease when plateauing) - Add block moves optimization that moves groups of dependent conditions together to escape local minima - Add cost-based tie-breaking using BddCostEstimator when multiple positions have the same node count * Implement rules engine ITE fn and S3 tree transform This commit adds a new function to the rules engine, ite, that performs an if-then-else check on a boolean expression without branching. By not needing to branch in the decision tree, we avoid SSA transforms on divergent branches which would create syntactically different but semantically identical expressions that the BDD cannot deduplicate. This commit also adds an S3-specific decision tree transform that canonicalizes S3Express rules for better BDD compilation: 1. AZ extraction: Rewrites position-dependent substring operations to use a single split(Bucket, "--")[1] expression across all branches 2. URL canonicalization: Uses ITE to compute FIPS/DualStack URL segments, collapsing 4 URL variants into a single template with {_s3e_fips} and {_s3e_ds} placeholders 3. Auth scheme canonicalization: Uses ITE to select sigv4 vs sigv4-s3express based on DisableS3ExpressSessionAuth The transform makes the rules tree ~30% larger but enables dramatic BDD compression by making URL templates identical across FIPS/DualStack/auth variants. Endpoints that previously appeared distinct now collapse into single BDD results, reducing nodes and results by ~43%. * Run type checking on BDD so type() works * Improve some loops * Attempt to fix the windows build * Optimize rules further Add nullable boolean transform. Improve variable and expression sharing. Add S3 BDD integration test. Add TreeMapper to simplify transforms. * Reduce SSA noise * Refactor S3 tree rewriter into composable transforms Split the monolithic S3TreeRewriter into three independent transforms: - S3AzCanonicalizerTransform: substring → split for AZ extraction - S3RegionUnifierTransform: unify Region/bucketArn#region/us-east-1 - S3ExpressEndpointTransform: canonicalize FIPS/DualStack/auth Added region unification pass that rewrites all region references to _effective_std_region (for aws-global → us-east-1 mapping) or _effective_arn_region (for UseArnRegion logic). This enables additional BDD sharing across endpoints with different region sources. S3 BDD stats: 77 conditions, 97 results, 484 nodes after sifting. * Address PR feedback on gradle and test case * Add BDD coverage testing and unref condition tx
1 parent a1a772f commit 4c97b7c

File tree

42 files changed

+3599
-908
lines changed

Some content is hidden

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

42 files changed

+3599
-908
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "feature",
3+
"description": "Implement rules engine ITE fn and S3 tree transform",
4+
"pull_requests": [
5+
"[#2903](https://github.com/smithy-lang/smithy/pull/2903)"
6+
]
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "feature",
3+
"description": "Improve BDD sifting (2x speed, more reduction)",
4+
"pull_requests": [
5+
"[#2890](https://github.com/smithy-lang/smithy/pull/2890)"
6+
]
7+
}

docs/source-2.0/additional-specs/rules-engine/standard-library.rst

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,103 @@ The following example uses ``isValidHostLabel`` to check if the value of the
208208
}
209209
210210
211+
.. _rules-engine-standard-library-ite:
212+
213+
``ite`` function
214+
================
215+
216+
Summary
217+
An if-then-else function that returns one of two values based on a boolean condition.
218+
Argument types
219+
* condition: ``bool``
220+
* trueValue: ``T`` or ``option<T>``
221+
* falseValue: ``T`` or ``option<T>``
222+
Return type
223+
* ``ite(bool, T, T)`` → ``T`` (both non-optional, result is non-optional)
224+
* ``ite(bool, T, option<T>)`` → ``option<T>`` (any optional makes result optional)
225+
* ``ite(bool, option<T>, T)`` → ``option<T>`` (any optional makes result optional)
226+
* ``ite(bool, option<T>, option<T>)`` → ``option<T>`` (both optional, result is optional)
227+
Since
228+
1.1
229+
230+
The ``ite`` (if-then-else) function evaluates a boolean condition and returns one of two values based on
231+
the result. If the condition is ``true``, it returns ``trueValue``; if ``false``, it returns ``falseValue``.
232+
This function is particularly useful for computing conditional values without branching in the rule tree, resulting
233+
in fewer result nodes, and enabling better BDD optimizations as a result of reduced fragmentation.
234+
235+
.. important::
236+
Both ``trueValue`` and ``falseValue`` must have the same base type ``T``. The result type follows
237+
the "least upper bound" rule: if either branch is optional, the result is optional.
238+
239+
The following example uses ``ite`` to compute a URL suffix based on whether FIPS is enabled:
240+
241+
.. code-block:: json
242+
243+
{
244+
"fn": "ite",
245+
"argv": [
246+
{"ref": "UseFIPS"},
247+
"-fips",
248+
""
249+
],
250+
"assign": "fipsSuffix"
251+
}
252+
253+
The following example uses ``ite`` with ``coalesce`` to handle an optional boolean parameter:
254+
255+
.. code-block:: json
256+
257+
{
258+
"fn": "ite",
259+
"argv": [
260+
{
261+
"fn": "coalesce",
262+
"argv": [
263+
{"ref": "DisableFeature"},
264+
false
265+
]
266+
},
267+
"disabled",
268+
"enabled"
269+
],
270+
"assign": "featureState"
271+
}
272+
273+
274+
.. _rules-engine-standard-library-ite-examples:
275+
276+
--------
277+
Examples
278+
--------
279+
280+
The following table shows various inputs and their corresponding outputs for the ``ite`` function:
281+
282+
.. list-table::
283+
:header-rows: 1
284+
:widths: 20 25 25 30
285+
286+
* - Condition
287+
- True Value
288+
- False Value
289+
- Output
290+
* - ``true``
291+
- ``"-fips"``
292+
- ``""``
293+
- ``"-fips"``
294+
* - ``false``
295+
- ``"-fips"``
296+
- ``""``
297+
- ``""``
298+
* - ``true``
299+
- ``"sigv4"``
300+
- ``"sigv4-s3express"``
301+
- ``"sigv4"``
302+
* - ``false``
303+
- ``"sigv4"``
304+
- ``"sigv4-s3express"``
305+
- ``"sigv4-s3express"``
306+
307+
211308
.. _rules-engine-standard-library-not:
212309

213310
``not`` function

smithy-aws-endpoints/build.gradle.kts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,60 @@ description = "AWS specific components for managing endpoints in Smithy"
1111
extra["displayName"] = "Smithy :: AWS Endpoints Components"
1212
extra["moduleName"] = "software.amazon.smithy.aws.endpoints"
1313

14+
// Custom configuration for S3 model - kept separate from test classpath to avoid
15+
// polluting other tests with S3 model discovery
16+
val s3Model: Configuration by configurations.creating
17+
1418
dependencies {
1519
api(project(":smithy-aws-traits"))
1620
api(project(":smithy-diff"))
1721
api(project(":smithy-rules-engine"))
1822
api(project(":smithy-model"))
1923
api(project(":smithy-utils"))
24+
25+
s3Model("software.amazon.api.models:s3:1.0.11")
26+
}
27+
28+
// Integration test source set for tests that require the S3 model
29+
// These tests require JDK 17+ due to the S3 model dependency
30+
sourceSets {
31+
create("it") {
32+
compileClasspath += sourceSets["main"].output + sourceSets["test"].output
33+
runtimeClasspath += sourceSets["main"].output + sourceSets["test"].output
34+
}
35+
}
36+
37+
configurations["itImplementation"].extendsFrom(configurations["testImplementation"])
38+
configurations["itRuntimeOnly"].extendsFrom(configurations["testRuntimeOnly"])
39+
configurations["itImplementation"].extendsFrom(s3Model)
40+
41+
// Configure IT source set to compile with JDK (17+) since the models it uses require it.
42+
tasks.named<JavaCompile>("compileItJava") {
43+
sourceCompatibility = "17"
44+
targetCompatibility = "17"
45+
}
46+
47+
val integrationTest by tasks.registering(Test::class) {
48+
description = "Runs integration tests that require external models like S3"
49+
group = "verification"
50+
testClassesDirs = sourceSets["it"].output.classesDirs
51+
classpath = sourceSets["it"].runtimeClasspath
52+
dependsOn(tasks.jar)
53+
shouldRunAfter(tasks.test)
54+
55+
// Pass build directory to tests
56+
systemProperty(
57+
"buildDir",
58+
layout.buildDirectory
59+
.get()
60+
.asFile.absolutePath,
61+
)
62+
}
63+
64+
tasks.test {
65+
finalizedBy(integrationTest)
66+
}
67+
68+
tasks.named("check") {
69+
dependsOn(integrationTest)
2070
}
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package software.amazon.smithy.rulesengine.aws.language.functions;
6+
7+
import java.io.IOException;
8+
import java.nio.file.Files;
9+
import java.nio.file.Path;
10+
import java.nio.file.Paths;
11+
import java.util.List;
12+
import org.junit.jupiter.api.BeforeAll;
13+
import org.junit.jupiter.api.Test;
14+
import software.amazon.smithy.model.Model;
15+
import software.amazon.smithy.model.node.Node;
16+
import software.amazon.smithy.model.shapes.ModelSerializer;
17+
import software.amazon.smithy.model.shapes.ServiceShape;
18+
import software.amazon.smithy.model.shapes.ShapeId;
19+
import software.amazon.smithy.rulesengine.analysis.BddCoverageChecker;
20+
import software.amazon.smithy.rulesengine.aws.s3.S3TreeRewriter;
21+
import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
22+
import software.amazon.smithy.rulesengine.language.evaluation.TestEvaluator;
23+
import software.amazon.smithy.rulesengine.logic.bdd.CostOptimization;
24+
import software.amazon.smithy.rulesengine.logic.bdd.SiftingOptimization;
25+
import software.amazon.smithy.rulesengine.logic.cfg.Cfg;
26+
import software.amazon.smithy.rulesengine.traits.EndpointBddTrait;
27+
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
28+
import software.amazon.smithy.rulesengine.traits.EndpointTestCase;
29+
import software.amazon.smithy.rulesengine.traits.EndpointTestsTrait;
30+
31+
class S3BddTest {
32+
private static final ShapeId S3_SERVICE_ID = ShapeId.from("com.amazonaws.s3#AmazonS3");
33+
private static Model model;
34+
private static ServiceShape s3Service;
35+
private static EndpointRuleSet originalRules;
36+
private static EndpointRuleSet rules;
37+
private static List<EndpointTestCase> testCases;
38+
39+
@BeforeAll
40+
static void loadS3Model() {
41+
model = Model.assembler()
42+
.discoverModels()
43+
.assemble()
44+
.unwrap();
45+
46+
s3Service = model.expectShape(S3_SERVICE_ID, ServiceShape.class);
47+
originalRules = s3Service.expectTrait(EndpointRuleSetTrait.class).getEndpointRuleSet();
48+
rules = S3TreeRewriter.transform(originalRules);
49+
testCases = s3Service.expectTrait(EndpointTestsTrait.class).getTestCases();
50+
}
51+
52+
@Test
53+
void compileToBddWithOptimizations() {
54+
// Verify transforms preserve semantics by running all test cases
55+
for (EndpointTestCase testCase : testCases) {
56+
TestEvaluator.evaluate(rules, testCase);
57+
}
58+
59+
// Build CFG and compile to BDD
60+
Cfg cfg = Cfg.from(rules);
61+
EndpointBddTrait trait = EndpointBddTrait.from(cfg);
62+
63+
StringBuilder sb = new StringBuilder();
64+
sb.append("\n=== BDD STATS ===\n");
65+
sb.append("Conditions: ").append(trait.getConditions().size()).append("\n");
66+
sb.append("Results: ").append(trait.getResults().size()).append("\n");
67+
sb.append("Initial BDD nodes: ").append(trait.getBdd().getNodeCount()).append("\n");
68+
69+
// Apply sifting optimization
70+
SiftingOptimization sifting = SiftingOptimization.builder().cfg(cfg).build();
71+
EndpointBddTrait siftedTrait = sifting.apply(trait);
72+
sb.append("After sifting - nodes: ").append(siftedTrait.getBdd().getNodeCount()).append("\n");
73+
74+
// Apply cost optimization
75+
CostOptimization cost = CostOptimization.builder().cfg(cfg).build();
76+
EndpointBddTrait optimizedTrait = cost.apply(siftedTrait);
77+
sb.append("After cost opt - nodes: ").append(optimizedTrait.getBdd().getNodeCount()).append("\n");
78+
System.out.println("Unreferenced BDD conditions before dead condition elimination: "
79+
+ new BddCoverageChecker(optimizedTrait).getUnreferencedConditions());
80+
81+
EndpointBddTrait finalizedTrait = optimizedTrait.removeUnreferencedConditions();
82+
System.out.println("Unreferenced BDD conditions after dead condition elimination: "
83+
+ new BddCoverageChecker(optimizedTrait).getUnreferencedConditions());
84+
85+
// Print conditions for analysis
86+
sb.append("\n=== CONDITIONS ===\n");
87+
for (int i = 0; i < finalizedTrait.getConditions().size(); i++) {
88+
sb.append(i).append(": ").append(finalizedTrait.getConditions().get(i)).append("\n");
89+
}
90+
91+
// Print results (endpoints) for analysis
92+
sb.append("\n=== RESULTS ===\n");
93+
for (int i = 0; i < finalizedTrait.getResults().size(); i++) {
94+
sb.append(i).append(": ").append(finalizedTrait.getResults().get(i)).append("\n");
95+
}
96+
97+
System.out.println(sb);
98+
99+
// Verify transforms preserve semantics by running all test cases on the BDD -and- ensuring 100% coverage.
100+
BddCoverageChecker coverageCheckerBdd = new BddCoverageChecker(finalizedTrait);
101+
for (EndpointTestCase testCase : testCases) {
102+
coverageCheckerBdd.evaluateTestCase(testCase);
103+
}
104+
105+
if (coverageCheckerBdd.getConditionCoverage() < 100) {
106+
throw new RuntimeException("Condition coverage < 100%: "
107+
+ coverageCheckerBdd.getConditionCoverage()
108+
+ " : " + coverageCheckerBdd.getUnevaluatedConditions());
109+
}
110+
111+
if (coverageCheckerBdd.getResultCoverage() < 100) {
112+
throw new RuntimeException("Result coverage < 100%: "
113+
+ coverageCheckerBdd.getResultCoverage()
114+
+ " : " + coverageCheckerBdd.getUnevaluatedResults());
115+
}
116+
117+
// Write model with BDD trait to build directory
118+
writeModelWithBddTrait(finalizedTrait);
119+
}
120+
121+
private void writeModelWithBddTrait(EndpointBddTrait bddTrait) {
122+
String buildDir = System.getProperty("buildDir");
123+
if (buildDir == null) {
124+
System.out.println("buildDir system property not set, skipping model output");
125+
return;
126+
}
127+
128+
// Create updated service with BDD trait instead of RuleSet trait
129+
ServiceShape updatedService = s3Service.toBuilder()
130+
.removeTrait(EndpointRuleSetTrait.ID)
131+
.addTrait(bddTrait)
132+
.build();
133+
134+
// Build updated model
135+
Model updatedModel = model.toBuilder()
136+
.removeShape(s3Service.getId())
137+
.addShape(updatedService)
138+
.build();
139+
140+
// Serialize to JSON
141+
ModelSerializer serializer = ModelSerializer.builder().build();
142+
String json = Node.prettyPrintJson(serializer.serialize(updatedModel));
143+
144+
// Write to build directory
145+
Path outputPath = Paths.get(buildDir, "s3-bdd-model.json");
146+
try {
147+
Path parentDir = outputPath.getParent();
148+
if (parentDir != null) {
149+
Files.createDirectories(parentDir);
150+
}
151+
Files.writeString(outputPath, json);
152+
System.out.println("Wrote S3 BDD model to: " + outputPath);
153+
} catch (IOException e) {
154+
throw new RuntimeException("Failed to write S3 BDD model", e);
155+
}
156+
}
157+
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package software.amazon.smithy.rulesengine.aws.language.functions;
6+
7+
import static org.junit.jupiter.api.Assertions.assertFalse;
8+
9+
import java.util.List;
10+
import org.junit.jupiter.api.BeforeAll;
11+
import org.junit.jupiter.api.Test;
12+
import software.amazon.smithy.model.Model;
13+
import software.amazon.smithy.model.shapes.ServiceShape;
14+
import software.amazon.smithy.model.shapes.ShapeId;
15+
import software.amazon.smithy.rulesengine.aws.s3.S3TreeRewriter;
16+
import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
17+
import software.amazon.smithy.rulesengine.language.evaluation.TestEvaluator;
18+
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
19+
import software.amazon.smithy.rulesengine.traits.EndpointTestCase;
20+
import software.amazon.smithy.rulesengine.traits.EndpointTestsTrait;
21+
22+
/**
23+
* Runs the endpoint test cases against the transformed S3 model. We're fixed to a specific version for this test,
24+
* but could periodically bump the version if needed.
25+
*/
26+
class S3TreeRewriterTest {
27+
private static final ShapeId S3_SERVICE_ID = ShapeId.from("com.amazonaws.s3#AmazonS3");
28+
29+
private static EndpointRuleSet originalRules;
30+
private static List<EndpointTestCase> testCases;
31+
32+
@BeforeAll
33+
static void loadS3Model() {
34+
Model model = Model.assembler()
35+
.discoverModels()
36+
.assemble()
37+
.unwrap();
38+
39+
ServiceShape s3Service = model.expectShape(S3_SERVICE_ID, ServiceShape.class);
40+
originalRules = s3Service.expectTrait(EndpointRuleSetTrait.class).getEndpointRuleSet();
41+
testCases = s3Service.expectTrait(EndpointTestsTrait.class).getTestCases();
42+
}
43+
44+
@Test
45+
void transformPreservesEndpointTestSemantics() {
46+
assertFalse(testCases.isEmpty(), "S3 model should have endpoint test cases");
47+
48+
EndpointRuleSet transformed = S3TreeRewriter.transform(originalRules);
49+
for (EndpointTestCase testCase : testCases) {
50+
TestEvaluator.evaluate(transformed, testCase);
51+
}
52+
}
53+
}

0 commit comments

Comments
 (0)