[Endpoints BDD 5/5] Add a result cache to the BDD endpoint provider - #7322
Open
alextwoods wants to merge 8 commits into
Open
[Endpoints BDD 5/5] Add a result cache to the BDD endpoint provider#7322alextwoods wants to merge 8 commits into
alextwoods wants to merge 8 commits into
Conversation
alextwoods
force-pushed
the
alexwoo/endpoints-bdd-pr4
branch
2 times, most recently
from
August 26, 2026 21:49
8f6d3cf to
17934a6
Compare
alextwoods
changed the base branch from
alexwoo/endpoints-bdd-pr4
to
feature/master/endpoints-bdd
August 27, 2026 18:47
Generate a single-entry (params -> endpoint) cache into each
Default{Service}EndpointProvider produced by the BDD codegen path, so a
client resolving the same endpoint repeatedly skips the BDD walk after
the first call. Scoped to the BDD path only; the rules2 path that every
shipped service uses is untouched, because no service model ships an
endpoint-bdd-1.json yet and that keeps a caching defect away from
customers while the approach is evaluated.
The cache is a volatile field holding an immutable CacheEntry. Racing
threads compute equivalent entries for equal params, so a lost write
costs one re-resolution and needs no further synchronisation. Only
successful resolutions are stored: a rule error or a no-match leaves the
previous entry in place, so a bad call neither poisons the cache nor gets
replayed from it.
Cache-key comparison is generated per parameter from a codegen-time
classification (EndpointCacheKeyClassification, computed by
EndpointProviderCacheIndex), ordered cheapest check first with an early
exit on mismatch:
BOOLEAN - identity, which is complete for Boolean rather than
merely fast, since autoboxing returns the TRUE and
FALSE singletons
CLIENT_STATIC_REF - identity (AWS::Region, clientContextParams)
OPERATION_STATIC - identity (staticContextParams literals)
SEMI_STABLE - identity then equals (SDK::Endpoint,
AccountIdEndpointMode)
IDENTITY_DERIVED - identity then equals (AWS::Auth::AccountId)
REQUEST_DYNAMIC - identity then equals (contextParam, JMESPath)
REQUEST_LIST - size-capped element-wise identity/equals
Classifications are read from the BDD model's parameters, not the rule
set's. The generated provider evaluates the BDD, nothing in codegen
enforces that the two files agree, and a parameter absent from the key is
the one defect here that returns an endpoint resolved for different
inputs. Codegen fails outright rather than skipping a parameter it
cannot place.
Reference stability, which raises the hit rate but is not required for
correctness since every string tier keeps an equals fallback:
- StaticClientEndpointProvider sanitizes the client endpoint once at
construction instead of rebuilding the URI per request, exposed through
a new ClientEndpointProvider#sanitizedEndpointString() that
AwsEndpointProviderUtils#endpointBuiltIn now delegates to. The
transform has a single definition so the cached and recomputed forms
cannot drift.
- AccountIdEndpointMode#endpointModeValue() returns an interned literal
from a field rather than name().toLowerCase(), and
EndpointParamsKnowledgeIndex emits it.
- EndpointResolverUtilsSpec hoists staticContextParams array values to
static final unmodifiable lists.
The last two also remove a per-request allocation on the rules2 path.
Testing:
- BddEndpointProviderCacheTest, 30 tests over the bddendpoints service:
one no-stale-hit test per parameter, hit assertions via instance
identity, unset transitions, equals fallbacks, the list size cap,
errors never cached, and 16-thread concurrent resolution of two
parameter sets. Mutation-checked: emptying the key fails 21 of 30, and
dropping only clientStringParam fails exactly the test that names it.
- EndpointProviderCacheIndexTest pins each parameter's tier, the
comparison order, and that classification reads the BDD rather than the
rule set. Tier assignment is invisible at runtime, so it is asserted
here or not at all.
- bddendpoints and the default-regional codegen models declare
parameters their BDD graphs never read, which is how all seven tiers
get covered without a BDD compiler: the node graph indexes conditions
rather than naming parameters, and the cache key spans every declared
parameter.
- queryServiceModelsWithBddEndpoints now pairs the S3 BDD with the S3
rule set instead of the four-parameter default-regional one, so the
17 parameters in the golden file match the params class.
Replace the seven-tier cacheParamsMatch with a uniform Objects.equals chain, ordered into three coarse groups: booleans, then strings whose reference the SDK keeps stable, then everything else, each group in the model's declaration order. Benchmarking says the tiers were not earning their complexity. Against this form they bought nothing on the hit path, which is the only path a cache exists to improve, and about 0.2 ns on the miss shape that motivates ordering at all - ahead of a ~1400 ns resolution. They cost a classification pass over every operation, a seven-value enum, and three different emitted code shapes. Full data, including why hashing the params would be worse than comparing them and why the comparison stays a private static method in the provider rather than moving onto the params class, is in .kiro/reference/endpoint_cache_key_benchmark.md. Objects.equals is what makes one emitter sufficient: it tries identity before equals, so a parameter whose reference is stable settles on the identity check and one that arrives fresh falls through and still matches. That was the tiers' main trick, available for free. Ordering survives because it is nearly free to derive - a parameter's group follows from its declared type plus whether it is AWS::Region or a client context param, with no analysis of the service's operations - and it is worth 20 ns on a miss against a late-declared boolean. It cannot affect correctness, since the chain compares every parameter before returning true. List parameters route through a generated cacheListsMatch helper instead of Objects.equals, keeping every term in the chain a single boolean expression and keeping the comparison bounded. List.equals is unbounded, and resolution is typically indifferent to list length, so an unbounded key check can cost more than the resolution it avoids and turn the cache into a pessimisation for that request shape. Above the cap the provider reports a miss and resolves, which is what it would have done anyway. The helper is only emitted when the model declares a stringArray. Deletes EndpointCacheKeyClassification, EndpointProviderCacheIndex and EndpointProviderCacheIndexTest. Testing: - The classification unit test is replaced by two assertions on the generated source, which is a stronger place to make them: that the key compares every parameter the BDD declares, and that the three-group ordering holds. The first is the invariant that matters - a parameter missing from the key returns an endpoint resolved for a different value of it - and it now covers all three BDD test models. - Mutation-checked both levels. Dropping a parameter that exists only in the runtime model fails exactly one of the 30 BddEndpointProviderCacheTest cases, the one that names it; dropping parameters present in the codegen models fails the completeness assertion, the ordering assertion and both golden files. - codegen 707 pass, codegen-generated-classes-test 3677 pass, checkstyle clean.
Two changes to the generated cache key, both driven by the per-service measurements in .kiro/reference/endpoint_cache_service_shapes.md. 1. Exclude parameters no condition and no result reads. A parameter nothing reads cannot change the resolved endpoint, so comparing it can only turn a hit into a miss that resolves to the endpoint already cached. S3 is why this matters. It declares Key, Prefix and CopySource, reads none of them, and binds Key as a contextParam - so Key changes on essentially every object request. With Key in the key, S3's cache misses on almost every GetObject and pays 6.4 ns per request for nothing. Dropping the three unread parameters takes the key from 17 comparisons to 14, makes a hit 41% cheaper on fresh references, and converts the dominant miss into a hit. 2. Compare only element 0 of a stringArray read only at index 0. When every read of a list is getAttr(list, "[0]"), nothing past the first element reaches the endpoint, so the rest cannot change the answer. DynamoDB is why this matters. It reads ResourceArnList only through getAttr(ResourceArnList, "[0]"), and comparing a freshly built three-element ARN list measured 15.5 ns against a 28 ns regional resolution - over half the cost the cache exists to avoid, on a latency path that matters. Comparing element 0 makes it O(1). This also makes an absent list and an empty one the same key, which is correct rather than a concession: the runtime's listAccess returns null for both, so both take the same branch during resolution. It is strictly more permissive than comparing whole lists, so it can only turn misses into hits. Detection is conservative in the safe direction. BddParameterReferences walks the conditions and results with the same parser the generator uses, and any read that is not an index-0 access - isSet, a template interpolation, a non-zero index, passing the list to a function - marks the parameter as needing a full comparison. Erring that way costs comparison work; erring the other way would drop something from the key that can change the endpoint. Testing: - BddParameterReferences is not tested directly. Both behaviours are asserted on the generated source and on runtime behaviour, because those are what can be wrong in a way that matters; a unit test over the usage map would restate the implementation. - The codegen tests assert the key covers every referenced parameter for all three BDD models, omits the unreferenced ones, routes a whole-list parameter and an index-0-only parameter to their respective helpers, and that the provider really does read only element 0 - so the comparison and the thing it depends on cannot drift apart. - The runtime suite grows to 38 tests: changing element 0 invalidates while changing a later element, shortening the list, or going far past the size cap all hit; an unread parameter never invalidates; and the whole-list parameter keeps the previous element-wise coverage. - Mutation-checked three ways. Misclassifying whole-list reads as index-0-only fails the 4 whole-list tests; treating every parameter as unreferenced fails 22; never detecting index-0-only fails the 4 first-element tests. The suite discriminates in both directions. - Both test models needed their extra parameters wired into the node graph. They had been declared but unreferenced, so they would now be correctly excluded and the tests covering them would assert nothing. Each new condition is a node whose branches share a successor, which makes it genuinely evaluated without changing what any request resolves to. codegen 709 pass, codegen-generated-classes-test 3685 pass, checkstyle clean.
…ison
The rules language requires a null check before an indexed access, so a
model that reads list[0] always reads isSet(list) as well. Counting that
null check as a whole-value read meant the first-element comparison never
fired on a real model: DynamoDB's ResourceArnList, the case it was added
for, was still compared element by element.
isSet observes only whether the parameter is present, so on its own it no
longer disqualifies the parameter. Verified against the real DynamoDB BDD:
the analysis now classifies ResourceArnList as FIRST_ELEMENT_ONLY and the
other eight parameters as FULL.
Presence does have to stay in the cache key, though, and that is a change
from the previous commit. Because isSet tells an absent list apart from an
empty one, the generated comparison now checks presence as well as element
0, rather than treating both as null:
if (a == b) return true;
if (a == null || b == null) return false;
String firstA = a.isEmpty() ? null : a.get(0);
String firstB = b.isEmpty() ? null : b.get(0);
return Objects.equals(firstA, firstB);
Collapsing absent and empty is sound only when the BDD's branches for the
two converge. DynamoDB's do - traced through its graph, cond20 false and
cond21 false both land on nodeP27, and likewise nodeP53 for the second
occurrence - so it would have been correct there. It is a property of the
graph rather than of the parameter, so relying on it would mean a future
model could quietly invalidate the comparison. One extra reference check
buys independence from that, and the comparison stays O(1).
Test model changes:
- Both BDD fixtures gained the isSet guard ahead of their index-0 access,
so they match the shape a real model produces. Without it the fixtures
were testing a case that cannot occur.
- The whole-list parameters were previously read only via isSet, which now
correctly qualifies them for the first-element comparison and left the
whole-list path uncovered. Each now also reads index 1, the smallest
realistic change that makes the rest of the list matter.
- firstElementList_emptyAndUnset_areTheSameKey becomes
..._areDistinguished, matching the new semantics.
- New codegen assertion that a list read past the head is compared in
full, since the boundary between the two helpers is a correctness line
rather than a preference.
- Mutation-checked the new logic: treating isSet as a whole-value read -
exactly the reported regression - fails three runtime tests, the
codegen assertion, and the golden file.
codegen 710 pass, codegen-generated-classes-test 3685 pass, checkstyle
clean.
alextwoods
force-pushed
the
alexwoo/endpoints-bdd-pr5
branch
from
August 27, 2026 19:29
22ee02a to
2b43c5c
Compare
It was public only because of a package boundary: the transformation lived on StaticClientEndpointProvider in core.internal, and the caller - ClientEndpointProvider's default method - sits in core, so nothing weaker than public could reach it. That is a poor reason for a public member, even on an @SdkInternalApi class. The transformation now lives in the interface default, which is the implementation every provider that does not override it already uses, and StaticClientEndpointProvider's constructor calls ClientEndpointProvider.super.sanitizedEndpointString() to compute the value it caches. One definition, as before, with nothing public added. The class is now final. That is what makes the constructor call provably safe: the super call is non-virtual, but the default it invokes reads clientEndpoint() and isEndpointOverridden(), and a subclass overriding either could have observed partial construction. Nothing subclasses it and it is @SdkInternalApi, so sealing it costs nothing and removes the hazard rather than documenting it. Verified the two implementations still agree: for a matrix of endpoints covering query parameters, user info, explicit ports, fragments and plain hosts, the caching implementation and the interface default produce the same string, the not-overridden case still yields null, and the caching one still returns an identical reference across calls. sdk-core 1505 + 624 pass, aws-core 317, codegen 710, codegen-generated-classes-test 3685, checkstyle clean.
alextwoods
marked this pull request as ready for review
August 27, 2026 20:25
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.
Summary
Adds a single-entry result cache to the BDD-generated endpoint provider. A client resolving
endpoints for the same configuration and request shape repeatedly re-walks the decision diagram
and rebuilds an identical
Endpointon every request. The provider now holds the last(params, endpoint)pair and returns the cached endpoint when the incoming params still match,turning a 27–346 ns resolution into a 1–5 ns comparison.
The cache is scoped to the BDD provider only. The rules2-generated provider is unchanged.
Caching an endpoint is only safe if the key covers everything that can change the answer, so most
of the change is in deriving that key at codegen time rather than in the cache itself. However, optimization to make the cache actually worthwhile involves limiting the cache key to as small a set as possible and optimizing the order to reduce the cost of cache misses.
The cache key
cacheParamsMatch is a flat Objects.equals chain, one term per parameter. Objects.equals tries identity before equals, so a parameter whose reference is stable across requests (a Region, a client context param) settles on the identity check, while one that arrives fresh falls through and still matches by value.
Only parameters the BDD actually references are used. A new codegen class, BddParameterReferences, walks the BDD's conditions and results with the existing expression parser and classifies each declared parameter as unreferenced, first-element-only, or fully read. Unreferenced parameters cannot affect the endpoint and are omitted. This matters most on S3, where Key, Prefix and CopySource are declared but read by zero rules — and Key changes on nearly every request. 14 of 17 S3 parameters are in the key.
List parameters are compared in bounded time. When the BDD only ever reads element 0 of a stringArray (DynamoDB's ResourceArnList), the key compares presence plus that one element and ignores the tail. Otherwise elements are compared pairwise up to a cap of 8, above which the provider reports a miss and resolves — List.equals is unbounded while resolution is largely indifferent to list length, so an uncapped key check could cost more than the work it avoids.
Terms are ordered booleans, then reference-stable strings, then the rest, each in declaration order. This only affects how quickly a mismatch is found; the chain compares every keyed parameter before returning true, so ordering cannot affect correctness.
Benchmarks
Changes affecting the non-BDD rules path
Three changes are not BDD-specific. They remove per-request allocations from the endpoint path and apply for all existing rules based services. Two exist because the cache benefits from stable references to compare cheaply, but each stands on its own as an allocation removal.
ClientEndpointProvider.sanitizedEndpointString()— new@SdkProtectedApidefault method returning the client endpoint with query and user-info stripped, which is what the rules engine receives asSDK::Endpoint.AwsEndpointProviderUtils.endpointBuiltIn()now delegates to it instead of constructing aURIand converting it to a string on every request.StaticClientEndpointProvideroverrides it with a value computed once at construction, so any client with an endpoint override drops aURIconstruction plus atoStringper request and returns a stable reference.AccountIdEndpointMode.endpointModeValue()— new method on this@SdkPublicApienum returning a per-constant interned literal. Generated code calledmode.name().toLowerCase(), allocating a fresh string per request for a value that has three possible results.EndpointResolverUtilsSpec** static list hoisting** —staticContextParamsarray values are emitted asprivate static final List<String>constants (STATIC_LIST_{OP}_{PARAM}) instead of being rebuilt inline, sosetStaticContextParamshands the builder a shared immutable list rather than an equal new one per request.License