Conversation
Follow-up to the Kafka header Base64-decode guard fix. Compares the hand-specialized GuardedBase64Decode/Breaker shape against a generic ParseHandler + Breaker abstraction (construction-time vs. call-time strategy composition) to see what devirtualization costs, if any, a reusable version of this pattern would carry. Motivated by APMLP-1513's children (APMLP-1760, APMLP-1769, APMLP-1772, APMLP-1783), which show enough independent exception/parsing-cost cases to justify considering a shared primitive rather than one-off copies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Replace the ParseHandler plus GenericBreakerCtor/GenericBreakerParam pair with one abstract DynamicLatch sketch, local to the benchmark, whose subclass is the strategy: an optimistic parse, a cheap correct pre-check and a stackless failure. That removes the constructor-versus-call-time question, since there is no separate strategy object. The latch has two flavors: get lets the failure flow to the caller (throwing a stackless stand-in while engaged), and tryGetOrNull converts it to null and builds no exception at all while engaged. Latches are static final fields of a named final subclass. The hand-written Breaker stays as the specialized baseline, and a setup self-check fails fast if the sketch misbehaves. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Rename the optimistic hook parse to handle, so it reads the same across the Latch family, and the pre-check isDefinitelyInvalid to isKnownToFail, which is not specific to parsers. Naming only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Record a five-fork run on Zulu 17 (M1) in the benchmark's Javadoc: the single-input arms and the mixed-input arms at five invalid rates. The sketch matches the hand-written breaker; the adaptive form is a tradeoff against always pre-checking at high invalid rates. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
What Does This Do
Benchmark-only. Reworks the exploratory
FunctionsBase64Benchmarkaround an abstractDynamicLatchsketch that is local to the benchmark (no production code changes).The earlier version compared the hand-written
Breakeragainst a genericParseHandlerplusGenericBreakerCtor/GenericBreakerParam(strategy in a constructor field versus passed at call time). Those two variants are replaced by one abstract class whose subclass is the strategy:handle(input): the operation done optimistically (for a parser, the parse), which may throw for bad input. Namedhandle, which matched the hook onLatchandClassLatchwhen this was written; those now useapply, and this sketch will be aligned whenDynamicLatchis revisited.isKnownToFail(input): a cheap, correct pre-check, true only ifhandlewould definitely fail; used while engaged.stacklessFailure(input): the failure to throw while engaged, with no stack trace.It has two public flavors, matching the
decode/decodeOrNullsplit in #12671:get(input): the failure flows to the caller. While engaged it throws the stackless stand-in instead of paying for a stack trace.tryGetOrNull(input): the failure becomesnull, and while engaged no exception is built at all.State is a plain countdown with hysteresis (engage on a failure of the declared type, disengage after 20 consecutive successes), as in #12671. Latches are
static finalfields of a named final subclass, one per arm. The hand-writtenBreakerstays as the specialized baseline, and the unguarded and pre-check arms are unchanged. TheMixarms drop the generic variants and gainmixLatchFlowandmixLatchConvert. A@Setupself-check fails fast if the sketch does not behave as the benchmark assumes.Motivation
Follow-up to #12671 (Kafka header Base64 guard). Enough independent exception and parsing-cost cases exist (APMLP-1760, APMLP-1769, APMLP-1772, APMLP-1783) to consider a shared primitive instead of repeating one-off copies. The design is tracked in APMLP-1884 (input-rate guard) and APMLP-1894 (the
Latchfamily:Latchin #12670,ClassLatchin #12702).DynamicLatchis the self-resetting member of that family;Breakeris reserved for dependency health.The constructor-versus-call-time question from the first version is resolved structurally: with an abstract class there is no separate strategy to wire. What remains to measure is the same question as APMLP-1893 (a
static finalsubclass versus a passed function).Additional Notes
Benchmark results. One run on Zulu 17.0.7 (HotSpot), MacBook M1, single thread, 5 forks, on a laptop with normal background activity (load about 4 to 7); the full tables are in the benchmark's Javadoc. JDK 8 and x86 are not measured. Error margins are below 4%.
Breaker, convertingBreaker, throwingDynamicLatch.tryGetOrNullDynamicLatch.getMix, ns/op, one invalid input in every N:
BreakerBreakerthrowinglatch.getlatch.tryGetOrNullThe sketch matches the hand-written breaker: within 0.2 ns in the single-input arms and within about 4 ns in the mixed ones (the largest gap is flow-through at one in 10,000, 39.6 against 35.3 ns). While engaged, the converting flavor costs about 2.8 ns where the status quo costs about 906 ns, and the flow-through flavor costs about 11.5 ns, the price of building a stackless exception. Always pre-checking more than doubles the cost of valid input (71 ns against 30 ns), which is what the adaptive form avoids.
It is a tradeoff, not a free win. With one invalid input in 100 or rarer, the guard costs 1 to 4 ns over doing nothing and is about 24 to 34 ns cheaper than always pre-checking. With a high rate (one in 2 or one in 10), always pre-checking is faster than the adaptive guard (36.6 ns against about 47 to 52 ns at one in 2, and 65 ns against about 70 to 72 ns at one in 10). I did not investigate why.
Not a proposal to merge the abstraction.
DynamicLatchcannot go intomainuntilLatchandClassLatchhave merged, and it needs at least two real users. This may be folded into Fix repeated exception cost from malformed Kafka header Base64 decoding #12671 later.Stackless failure gotchas (recorded in the benchmark's Javadoc and APMLP-1884):
FastFailBase64Exceptionis a new instance per failure, never shared. A shared instance would also have to disable suppression (otherwiseaddSuppressedaccumulates on it forever) and forbidinitCause.IllegalArgumentExceptionhas no constructor that turns the stack trace off, so the stand-in overridesfillInStackTrace.Declared failure type. The converting flavor must not swallow unrelated runtime exceptions, so the sketch takes a
Class<X>and converts only that type. Whether that should be a class token or a predicate is an open question in APMLP-1884.The branch is 24 commits behind master; I have not merged it.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: APMLP-1884
🤖 Generated with Claude Code