Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Cache regex assertion patterns safely - #10661

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions
Aug 24, 2026
Merged

Cache regex assertion patterns safely#10661
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-regex-assertions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • reuse string-pattern regular expressions through a private, bounded cache
  • keep cache hits lock-free while synchronizing only duplicate-check and insertion
  • preserve the caller-supplied Regex overload path and existing exception/telemetry ordering

Memory and behavior safety

The cache is a fixed 15-slot FIFO ring, matching the runtime's default static regex cache size. Only patterns up to 512 UTF-16 code units are admitted, so retained pattern text and regex count are both bounded; larger patterns are constructed per call and never retained. High-cardinality input overwrites old slots rather than growing process-lifetime state.

Entries are keyed by ordinal pattern text and the current culture name because default case-insensitive regex behavior captures culture when the Regex is constructed. Default options and timeout semantics remain those of new Regex(pattern). Construction remains in ToRegex, before assertion telemetry and value validation, preserving invalid-pattern/null ordering and exception stack shape. The overloads accepting a caller-created Regex bypass this cache unchanged.

Concurrent hits use volatile reads. Misses construct outside the lock, then perform a synchronized second lookup and fixed-slot insertion, so unrelated regex parsing is not serialized and concurrent callers converge on one cached instance.

Benchmarks

Independent Release microbenchmark, seven runs with median reported, telemetry opted out. The baseline mirrors the previous new Regex(pattern).IsMatch(value) path; the candidate invokes the public string assertion overload.

Runtime / scenarioBaselineCandidateAllocated baselineAllocated candidate
net8.0, 500k repeated1,221 ms128 ms1,684 MB0 B
net9.0, 500k repeated1,227 ms134 ms1,692 MB0 B
net8.0, 50k unique60 ms91 ms114.4 MB116.4 MB
net9.0, 50k unique59 ms87 ms115.2 MB117.2 MB

The repeated-pattern case is about 9–10x faster and allocation-free after warmup. The deliberately adversarial unique-pattern case pays the expected bounded lookup/insertion cost (about 27–31 ms and 2 MB across 50,000 calls) while a second 50,000-pattern sweep retained only the fixed cache footprint (approximately 240 bytes net measured growth after full GC).

Validation

  • full TestFramework.UnitTests build and execution on net48, net8.0, net9.0, and net8.0-windows10.0.18362.0
  • focused coverage for reuse, FIFO eviction, long-pattern bypass, culture separation, concurrent convergence, invalid-pattern/null ordering, and caller-supplied Regex bypass
  • two independent memory/concurrency/API reviews, followed by two post-fix reviews
  • binary log captured at artifacts/log/Debug/Build.binlog

Closes#10659

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 21, 2026 02:46
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 21, 2026
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed
Comment threadsrc/TestFramework/TestFramework/Assertions/Assert.Matches.cs Fixed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounded, culture-aware regex cache to improve repeated string-pattern assertion performance.

Changes:

  • Adds a lock-free-read, synchronized-write FIFO cache.
  • Adds tests for reuse, eviction, culture, concurrency, and bypass behavior.
Show a summary per file
FileDescription
src/TestFramework/TestFramework/Assertions/Assert.Matches.csImplements bounded regex caching.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.csAdds cache behavior tests.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 21, 2026 03:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/TestFramework/TestFramework/Assertions/Assert.Matches.cs:213

  • Keying only by CurrentCulture.Name does not fully identify the culture whose casing rules new Regex(pattern) captures. CultureInfo.Name and TextInfo are virtual on .NET Framework, so a valid derived/custom culture can retain the same name as a previously cached culture while supplying different casing rules; an inline (?i) pattern then reuses the wrong Regex, changing assertion results from the previous per-call construction. Preserve the bounded cache but key by the captured culture identity/casing semantics (or bypass caching for custom/derived cultures), and cover two same-name cultures with different TextInfo behavior.
 string cultureName = CultureInfo.CurrentCulture.Name;
if (RegexCache.TryGet(pattern, cultureName, out Regex cachedRegex))
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10661

Parallelization

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.VisualStudio.TestPlatform.TestFramework.UnitTestsoff (TestContainer engine — no MSTest parallel scheduler)n/an/a

⚠️AssertTests (and its partials, including the changed AssertTests.MatchesRegex.cs) derives from TestContainer (test/Utilities/TestFramework.ForTestingMSTest), a bespoke engine with no parallel scheduler at all. Every finding below is readiness-only — what would matter if this suite were ever ported to MSTest and opted into [Parallelize]. Nothing here is a live race today.

Findings: A (global-state) 1 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 2.

Top actions (by expected value):

  1. No action required today; the notes below are forward-looking readiness observations only.

Info

  • [A · High confidence]test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.MatchesRegex.cs:82-99 (MatchesRegex_WithCultureSensitivePattern_DoesNotReuseRegexAcrossCultures) — sets the process-wide-flowing CultureInfo.CurrentCulture (not DefaultThreadCurrentCulture). On the suite's actual TFMs (net48, net8.0, net9.0 — all ≥ .NET Framework 4.6 / modern .NET) this value is carried via ExecutionContext and does not leak to concurrently-running siblings, so it is not a live-race candidate even in a hypothetical parallel port — and the test already restores the original culture in a finally block, which is the correct pattern. Recorded for completeness; no fix needed.
  • [A · Medium confidence]src/TestFramework/TestFramework/Assertions/Assert.Matches.cs — the new RegexCache is a genuine process-global mutable static (a fixed-size ring buffer) that every test calling Assert.MatchesRegex(string, ...) / the new ToRegex reads and writes, including several of the newly added tests (MatchesRegex_WithRepeatedStringPattern_ReusesRegex, ..._WhenOldestRegexIsReusedBeforeCapacityIsExceeded..., ..._WithMaximumLengthStringPattern_ReusesRegex, etc.). If this suite were ever moved onto MSTest with [Parallelize], concurrent writers could evict each other's entries — but eviction only causes a cache miss (a fresh, functionally-identical Regex is compiled and returned), never an incorrect result, and the guid/prefix-randomized patterns used by these tests avoid cross-test key collisions. So this is not a correctness hazard under parallelism, only a (currently moot) contention/perf note; no [ResourceLock] or isolation is warranted. Cross-ref detect-static-dependencies/test-anti-patterns if a static-coupling concern is separately of interest — this audit's read is purely about race-safety, and there is none here.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 90.2 AIC · ⌖ 3.82 AIC · ⊞ 24.8K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10661

Reviewed the 12 new test methods added to AssertTests.MatchesRegex.cs covering the new bounded regex cache (BoundedRegexCache) in Assert.Matches.cs. All tests independently target a distinct, well-chosen mutation point (FIFO eviction vs. LRU, ordinal vs. ordinal-ignore-case key comparison, exact boundary at MaximumCachedRegexPatternLength, culture-sensitive key, concurrent insert convergence, and validation-order for null value vs. invalid pattern), and every one killed its target mutation in this review. Several tests (ReusesRegex, DoesNotReuseRegex, StillEvictsOldestRegex, boundary tests, culture test, concurrency test, BypassesStringPatternCache) reach into the private ToRegex/BoundedRegexCache implementation via reflection to observe cache identity, which is white-box coupling — but it's a reasonable trade-off here since cache reuse isn't observable through the public API any other way, so it does not pull any test below an A band. No high-confidence actionable findings were found, so no inline suggestions were posted.

GradeTestMutationNotesHow to improve
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins that pattern validation runs before the null-value check via the public API.
A (90–100)new AssertTests.
DoesNotMatchRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedKills a missing/mis-ordered null-value guard using only the public API.
A (90–100)new AssertTests.
MatchesRegex_
WithCaseDistinctStringPatterns_
DoesNotReuseRegex
1/1 killedPins ordinal, case-sensitive cache-key comparison.
A (90–100)new AssertTests.
MatchesRegex_
WithConcurrentCandidateRegexes_
ConvergesOnSingleCachedRegex
1/1 killedVerifies the synchronized second lookup dedupes racing inserts; relies on reflection into the private cache type.
A (90–100)new AssertTests.
MatchesRegex_
WithCultureSensitivePattern_
DoesNotReuseRegexAcrossCultures
1/1 killedUses the real tr-TR/en-US dotless-I distinction to prove culture is part of the cache key.
A (90–100)new AssertTests.
MatchesRegex_
WithInvalidStringPatternAndNullValue_
ThrowsPatternExceptionFirst
1/1 killedPins parameter-validation order via the public API only.
A (90–100)new AssertTests.
MatchesRegex_
WithMaximumLengthStringPattern_
ReusesRegex
1/1 killedExercises the exact 512-char boundary, complementing the over-limit test to pin the off-by-one.
A (90–100)new AssertTests.
MatchesRegex_
WithOverMaximumLengthStringPattern_
DoesNotCacheRegex
1/1 killedConfirms over-limit patterns bypass caching entirely.
A (90–100)new AssertTests.
MatchesRegex_
WithRegexPattern_
BypassesStringPatternCache
1/1 killedProves the Regex-overload path neither reads nor writes the string-pattern cache.
A (90–100)new AssertTests.
MatchesRegex_
WithRepeatedStringPattern_
ReusesRegex
1/1 killedKills the "cache never hits" mutation via reference-identity check.
A (90–100)new AssertTests.
MatchesRegex_
WithValidStringPatternAndNullValue_
ThrowsAssertFailedException
1/1 killedConfirms the null-value guard is not accidentally removed for the string-pattern overload.
A (90–100)new AssertTests.
MatchesRegex_
WhenOldestRegexIsReusedBeforeCapacityIsExceeded_
StillEvictsOldestRegex
1/1 killedFills capacity, re-touches the oldest entry, then inserts once more, correctly distinguishing FIFO from LRU.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 116.2 AIC · ⌖ 4.82 AIC · ⊞ 16.9K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit e287fd7 into mainAug 24, 2026
55 of 57 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-regex-assertions branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Cache compiled Regex instances in Assert.MatchesRegex/DoesNotMatchRegex

3 participants

@Evangelink@0101