Repository navigation
feat: alt-retry journaling for LOOP_ALT chains (v2-gamma), SQL_MYSQL chain win restored - #128
Merged
Merged
Conversation
Re-admits retryable LOOP_ALT bodies into the deterministic-chain family: bodies where one alternative can match a proper prefix of another ((?:a|ab)+, MySQL's \\" | [^"] escaped literals) now journal their chosen body per iteration and the give-back handler re-enters the consume loop at the popped iteration with a floor past the committed body - JDK backtracking order (retry the last iteration's later bodies first; a floor-gated consume failure flows to consumeEnd and re-runs the tail at the shortened boundary = the iteration's exit choice; the next pop retries the previous iteration). The min floor is enforced by the re-entered consumeEnd min check, which preserves the JDK distinction between retrying an iteration's alternatives and dropping the iteration. Journal entries encode (end << 4) | bodyIndex (MAX_CHAIN_LIT_ALT <= 16). Non-overlapping give-back loops keep the v2-beta boundary-only handler and plain journal values, so existing admitted patterns (SQL_ANSI, SQL_POSTGRESQL, Ldap, XmlTags) emit the same retry logic as before (the journal base indirection adds only constant-zero adds). Also fixes a latent v2-beta boundary-clobber: sequential give-back LOOP_ALTs in one generated method (MySQL's two quote-loop branches) both journaled from index 0, so the second loop's entries overwrote the first's - each give-back loop now journals into its own (len+1) slice of the per-thread scratch buffer (single-loop methods keep the original size). The analyzer no longer rejects overlapping bodies; checkChainDisjoint forces giveBack for them instead (a disjoint mixed-width loop's maximal end is not input-determined without body retry, e.g. (?:ab|a)+[bc] on "aab"). Verified: 263,821-input brute-force parity vs JDK across the retryable, star, forced-giveBack, sequential-slice, and MySQL shapes (0 divergences); budget delegation on the exponential (?:a|aa)+ shape intact. SQL_MYSQL routes back to DETERMINISTIC_CHAIN. Full suite + fuzz gate green. Tests: loopAltAltRetryParity (incl. 3,906-input exhaustive sweep), loopAltDisjointMixedWidthForcedGiveBackParity, loopAltSequentialGiveBackJournalSlicesParity, loopAltAltRetryOverflowDelegatesToPikeVM, loopAltOverlappingBodiesAdmittedWithAltRetry; sqlMysql tests restored to chain routing and chain parity with retry-triggering inputs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
========================================
Coverage 84.9% 84.9%
Complexity 1 1
========================================
Files 159 159
Lines 46752 46878 +126
Branches 6462 6485 +23
========================================
+ Hits 39698 39812 +114
- Misses 5096 5108 +12
Partials 1958 1958
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The packed journal removes high position bits at input offset 2^28, so retry can write past the journal on a valid large string. The new slice count also keeps one full-input slice for each separate branch, which can cause an avoidable heap failure.
🤖 Datadog Autotest · Commit f7c292b · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7c292b261
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…try lengths Addresses both PR review findings (Codex + Datadog Autotest, P2/P2): Journal slices now bound the maximum simultaneously-live path, not the syntactic sum. The constructor counted every give-back LOOP_ALT in the tree - including loops in mutually exclusive top-level branches and alternation bodies - so a legal 32-branch pattern with one loop per branch requested 32x(len+1) ints (~1 GiB at 8M chars) before trying any branch, retained per thread by chainScratch. Only one exclusive path can have a live retry chain at a time, and a failed branch's journal is dead once its retry chain unwinds into the next branch, so slice ordinals are now reused across exclusive paths: the top-level branch loops and the ALT_CHAIN body loop save/restore the EmitCtx ordinal counter (an ALT_CHAIN ends at the deepest body's span so a sequential give-back loop in the shared rest still stacks on the winning body's loops), and the journal is sized by maxPathLoopAltGiveBack (sequential elements and OPT/CAPTURE nested regions stack; alternation bodies take the max). This also halves SQL_MYSQL's journal: its two quote loops live in exclusive alternation branches and now share one (len+1) slice. The alt-retry journal entry packs (end << 4) | bodyIndex, which only represents ends below 1 << 27: at or above that the int shift discards the high bits and the give-back decode restores wrapped (wrong) positions. The "the journal would not fit memory" argument was wrong - a 2^28-int journal (~1 GiB) fits large heaps, so the case is reachable. Entry points that acquire a journal now guard on len >= 1 << 27 for alt-retry patterns and delegate the whole call to the PikeVM fallback (full-width positions, linear) before allocating. Boundary-only journals (SQL_ANSI etc.) store plain ends and keep the unguarded path. Verified: the 263,821-input brute-force parity sweep is unchanged (0 divergences); reflection-level sizing assertions (single loop 1, exclusive branches share 1, sequential loops stack 2, ALT_CHAIN + successor stack 2, SQL_MYSQL 1) and a 1,555-input exclusive-branch journal-reuse parity sweep; the 2^27+10-char SQL_MYSQL probe delegates (fallbackCount 0 -> 1) with the JDK answer while normal-size inputs stay on the chain path. Full suites + fuzz gate green.
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
2 participants
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.
Follow-up to #127
#127 deliberately declined retryable
LOOP_ALTbodies into the deterministic-chain family ((?:a|ab)+-style alternation loops where one body can match a proper prefix of another — including SQL_MYSQL's escaped-literal bodies\\" | [^"]), and accepted the resulting SQL_MYSQL tradeoff (~0.44x onSqlMysqlFind). This PR re-admits those shapes with exact JDK backtracking order.How it works
(end << 4) | bodyIndex(4 bits for the body,MAX_CHAIN_LIT_ALT <= 16).consumeEnd, which re-runs the tail at the shortened boundary — exactly the iteration's exit choice — and the next pop retries the previous iteration. The min floor is enforced by the re-enteredconsumeEndcheck (retrying the last iteration's alternatives atjc == minis allowed; dropping below min is not), which is precisely the JDK distinction between retrying an iteration and dropping an iteration.Latent v2-beta bug fixed alongside
Two sequential give-back
LOOP_ALTs in one generated method (SQL_MYSQL's two quote-loop branches) both journaled from index 0 — the second loop clobbered the first's boundaries. Each give-back loop now journals into its own(len+1)slice of the per-thread scratch buffer; single-loop methods keep the original allocation size.Analyzer
The body-overlap rejection is removed;
checkChainDisjointinstead forcesgiveBackfor overlapping bodies — a disjoint mixed-width loop's maximal end is not input-determined without body retry (e.g.(?:ab|a)+[bc]on"aab").Correctness verification
q(?:a|ab)+c\band the star variant (exhaustive over {q,a,b,c,x}^≤6), disjoint-mixed-width forced-give-back shapes, sequential-slice shapes (a(?:b|bc)*x(?:d|de)*zexhaustive), doubled quote loops, and the full SQL_MYSQL pattern on crafted literal inputs.q(?:a|aa)+zon 40 a's) trips the per-retry budget charge and delegates to PikeVM with the correct answer — same accounting envelope as all existing retryable constructs.Benchmarks (JDK 21 Temurin, -wi 5 -i 5 -f 3, workspace-jb)
SqlMysqlFind vs RE2J: 179x / 103x / 100x. All previously-won benchmarks held within noise — the journal-slice base adds only constant-zero adds on the non-overlapping paths.