Overhaul when/where we check for Regex timeouts

[stephentoub]

Regex timeouts have never been about guaranteeing exact timeout handling; rather, they're about avoiding catastrophic backtracking. As such, we already allow an O(n) amount of work in many cases between timeout checks. This change formalizes that, such that we now check for a timeout at every place where we could do at least an O(n) amount of work, which essentially means every time we match at a new index and every time we backtrack. It also removes the counting logic that previously translated only 1 out of 1000 CheckTimeout calls into a timeout check; now every CheckTimeout will query the current tick count.

This will fail CI until #68138 is merged.

(This does not change how we do timeout checks in the NonBacktracking implementation. Based on the above criteria, timeout checks in NonBacktracking are optional. However, we'll likely want to continue doing them periodically, for consistency and some level of predictability.)

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Regex timeouts have never been about guaranteeing exact timeout handling; rather, they're about avoiding catastrophic backtracking. As such, we already allow an O(n) amount of work in many cases between timeout checks. This change formalizes that, such that we now check for a timeout at every place where we could do at least an O(n) amount of work, which essentially means every time we match at a new index and every time we backtrack. It also removes the counting logic that previously translated only 1 out of 1000 CheckTimeout calls into a timeout check; now every CheckTimeout will query the current tick count.

This will fail CI until #68138 is merged.

(This does not change how we do timeout checks in the NonBacktracking implementation. Based on the above criteria, timeout checks in NonBacktracking are optional. However, we'll likely want to continue doing them periodically, for consistency and some level of predictability.)

Author:	stephentoub
Assignees:	stephentoub
Labels:	area-System.Text.RegularExpressions
Milestone:	-

[joperezr]

I suppose you have run them already, but can you share some perf numbers from our existing benchmarks to see how much (if any) impact (positive or negative) these changes around timeout might have?

[stephentoub]

I suppose you have run them already

I ran a worst-case test that would cause us to perform a timeout check at essentially every step, and I saw no measurable impact from calling Environment.TickCount64 at each check rather than maintaining a counter and calling Environment.TickCount 1/1000 such checks; TickCount{64} is cheap, and the other costs involved simply dominate. This also removes the checks on paths that have less overhead and are hotter, e.g. as part of doing the initial linear match of a loop, so all of the timeout overhead goes away in those cases.

[joperezr]

Minor comments but looks good otherwise. We will probably want to update our docs in dotnet/docs as well as dotnet/dotnet-api-docs once this goes in.