perf: cache compiled dynamic regexps in JRT; align special-variable check style#514
Conversation
Dynamic regexps reaching JRT.matches, matchPosition, replaceFirst/ replaceAll and the regexp paths of splitTokenizer were compiled with Pattern.compile on every invocation, so a gsub(dynstr, ...) loop reparses the same expression once per record. Cache compiled patterns by expression text next to the IGNORECASE pattern-twin cache, and revalidate the cached flags on access so IGNORECASE toggles still take effect immediately. The cache is dumped wholesale past 256 entries to keep scripts that synthesize regexps from input data bounded. style: make isJrtManagedSpecialVariable a switch like its sibling applySpecialVariable. Items 1 and 2 of #511; item 3 stays blocked on the Java 8 target and item 4 was deliberately left as-is. Closes #511 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 3c85a8ad0d
ℹ️ 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".
…flags() Pattern.flags() reflects inline flag constructs such as (?i), so a pattern like a(?i)b compiled under IGNORECASE=0 already reports CASE_INSENSITIVE and the staleness check reused it after toggling IGNORECASE=1, leaving the leading a case-sensitive. Give each IGNORECASE setting its own cache map instead of inspecting the compiled pattern. Addresses PR #514 review feedback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Implements the actionable items of #511 (post-#504 minor cleanups):
Item 2 — dynamic (string) regexp cache.
JRT.matches,matchPosition,replaceFirst/replaceAll, and the regexp paths ofsplitTokenizercompiled dynamic string patterns withPattern.compile(ere, regexpFlags())on every invocation, so agsub(dynstr, ...)loop reparsed the same expression once per record. A string-keyed cache now slots next toJRT.caseAwarePattern, as suggested in the issue. Cached entries revalidate their flags on access, soIGNORECASEtoggles still take effect immediately; the cache is dumped wholesale past 256 entries so scripts that synthesize regexps from input data stay bounded.Item 1 — style consistency.
isJrtManagedSpecialVariableis now aswitchlike its siblingapplySpecialVariable.Items 3 and 4 — intentionally not done. The
gensubStringBuffer(item 3) stays blocked whilemaven.compiler.releaseis 8 (Matcher.appendReplacement(StringBuilder, ...)is Java 9+), and the redundant subarray re-check in the sort tie-break (item 4) was already judged not worth the tangle in the issue itself.Verification
mvn verifypasses: 515 unit tests, 0 failures; Checkstyle/PMD/SpotBugs clean.IGNORECASE0 → 1 → 0 behaves correctly through the cache,split()with a regexp separator works, and 300 distinct dynamic patterns churn through the bounded cache without error.Closes #511
🤖 Generated with Claude Code