Support default timeouts in RegexGenerator #67775
Conversation
If you pass a timeout to Regex's constructor, that's the timeout that's used, and if it's infinite, no timeout is applied. However, if you don't specify a timeout, then while it defaults to infinite, it's also overridable by a process-wide AppContext switch. An AppContext value can be set before any Regex usage, and then any regexes that don't specify a timeout will default to that process-wide timeout value rather than to infinite. With RegexGenerator, the code is all generated ahead of time, before there's even a process in which to check for a setting, as such, up until now not specifying a timeout has been the same as explicitly specifying infinite. With this PR, we fix that. If you explicitly specify a timeout, infinite or otherwise, nothing changes. But if you don't specify a timeout, we now still emit the timeout related checks, but guarded by a read of a static readonly bool that's initialized with a read for that same AppContext switch. This allows for the generated code to still participate in the global default while allowing the JIT to remove the guarded blocks when the code is tiered up and it observes that the readonly static highlights there's no default set (in the vast majority case). And if someone really doesn't want that code emitted at all, they can simplify specify an infinite timeout in the RegexGenerator attribute.
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue DetailsIf you pass a timeout to Regex's constructor, that's the timeout that's used, and if it's infinite, no timeout is applied. However, if you don't specify a timeout, then while it defaults to infinite, it's also overridable by a process-wide AppContext switch. An AppContext value can be set before any Regex usage, and then any regexes that don't specify a timeout will default to that process-wide timeout value rather than to infinite. With RegexGenerator, the code is all generated ahead of time, before there's even a process in which to check for a setting, as such, up until now not specifying a timeout has been the same as explicitly specifying infinite. With this PR, we fix that. If you explicitly specify a timeout, infinite or otherwise, nothing changes. But if you don't specify a timeout, we now still emit the timeout related checks, but guarded by a read of a static readonly bool that's initialized with a read for that same AppContext switch. This allows for the generated code to still participate in the global default while allowing the JIT to remove the guarded blocks when the code is tiered up and it observes that the readonly static highlights there's no default set (in the vast majority case). And if someone really doesn't want that code emitted at all, they can simplify specify an infinite timeout in the RegexGenerator attribute. Fixes #59491
|
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
| { | ||
| if (RegexHelpers.IsNonBacktracking(engine)) | ||
| { | ||
| // test relies on backtracking taking a long time |
There was a problem hiding this comment.
we appear to have no tests that verify that the non backtracking engine respects the appdomain setting, although it appears it will.
...ries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorHelper.netcoreapp.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
|
Regressions on Windows x64: dotnet/perf-autofiling-issues#4611 |
ghost
locked as resolved and limited conversation to collaborators
4 participants
If you pass a timeout to Regex's constructor, that's the timeout that's used, and if it's infinite, no timeout is applied. However, if you don't specify a timeout, then while it defaults to infinite, it's also overridable by a process-wide AppContext switch. An AppContext value can be set before any Regex usage, and then any regexes that don't specify a timeout will default to that process-wide timeout value rather than to infinite.
With RegexGenerator, the code is all generated ahead of time, before there's even a process in which to check for a setting, as such, up until now not specifying a timeout has been the same as explicitly specifying infinite. With this PR, we fix that. If you explicitly specify a timeout, infinite or otherwise, nothing changes. But if you don't specify a timeout, we now still emit the timeout related checks, but guarded by a read of a static readonly bool that's initialized with a read for that same AppContext switch. This allows for the generated code to still participate in the global default while allowing the JIT to remove the guarded blocks when the code is tiered up and it observes that the readonly static highlights there's no default set (in the vast majority case). And if someone really doesn't want that code emitted at all, they can simplify specify an infinite timeout in the RegexGenerator attribute.
Fixes #59491
cc: @joperezr, @GrabYourPitchforks