Repository navigation
Soft-hold shared NFA DFA bundles to bound cache retention - #140
Conversation
PikeVMEntry/BitStateEntry/HybridEntry keep their shared DfaBundle / reject bundle behind a SoftReference: fixed-capacity LazyDFACache arrays per cached pattern (~40-70 KiB, several MiB populated) previously piled up unbounded in PIKEVM_NFA_CACHE/BITSTATE_NFA_CACHE. Bundles rebuild deterministically after eviction; live matchers are unaffected (they copy bundle fields). Hybrid nfa-half bundles are now shared across compiles (-80% compile time for hybrid-pikevm patterns); ineligible reject-DFA NFAs skip the per-matcher rebuild scan.
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
=======================================
Coverage 84.9% 84.9%
Complexity 1 1
=======================================
Files 163 163
Lines 48981 49002 +21
Branches 7000 7009 +9
=======================================
+ Hits 41594 41613 +19
- Misses 5378 5379 +1
- Partials 2009 2010 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![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: 0fd855a62e
ℹ️ 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".
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
For BITSTATE_CAPTURE patterns whose reject over-approximation accepts empty, the new ineligibility flag does not prevent matcher construction from rebuilding the reject DFA, so each compile still incurs the scan this change intends to eliminate.
🤖 Bits Code Review · Commit 0fd855a · @DataDog review to ask questions
…E sentinel Matchers retain the shared bundle they were built from, so the entry's SoftReference cannot be GC-cleared while live matchers still pin its caches (a cleared ref would allocate a duplicate bundle over caches the matchers already hold). RejectDfaFactory.NONE marks a known-ineligible NFA so BitStateMatcher skips its matcher-private build retry - the O(states) ineligibility scan now runs once per cache entry, not once per matcher.
2 participants
What does this PR do?
Bounds the retention of the NFA-derived DFA bundles shared per cached pattern:
PikeVMEntry.dfaBundle,BitStateEntry.rejectBundle, and the hybrid nfa-half bundles are now held throughSoftReference(deterministic rebuild after eviction) instead of strong fields. Also shares the hybrid nfa-half bundles across compiles (previously one private bundle percompile()call) and pins the reject-bundle negative result (NFAs whereRejectDfaFactory.buildreturns null no longer re-run the O(states) scan per matcher).Motivation
Review comment on #139: each bundle eagerly allocates fixed-capacity
LazyDFACachebacking arrays (~40–70 KiB per cache before population, several MiB once populated), and the long-livedPIKEVM_NFA_CACHE/BITSTATE_NFA_CACHEretained one strongly per cached pattern — a high-cardinality pattern workload could exhaust the heap, invisible tocacheSize()(which only countsPATTERN_CACHE). Soft retention keeps the sharing benefit while memory is plentiful and lets the GC evict under pressure.Related Issue(s)
Follow-up to #139 (review thread on
RuntimeCompiler.java).Change Type
Checklist
./gradlew build)Performance Impact
Benchmarks on workspace-jb (x86_64, 16 cores, Zulu 26), HEAD vs this change:
Compile per op (min of 5×50k, ns/op):
Match time (JMH
AllStrategyVsJdkBenchmark, pikevmCapture/lazyDfa, all scales): unchanged within error. Match is unaffected by design — matchers copy bundle fields into their own final state, so eviction never reaches a live matcher.Additional Notes
SharedDfaBundleTest(\b(a?)+x→ PIKEVM_CAPTURE,\b(a)+x→ BITSTATE_CAPTURE; both install a clearedSoftReferenceinto the real cache entry and assert correct rebuild + matching).volatilepublication is unchanged.cacheSize()blind spot is documented in the entry comments; extendingcacheSize()semantics is deliberately out of scope here.