Determine if/how to handle default timeouts in RegexGenerator

[stephentoub]

With the regex source generator merged in #59186, a developer can specify timeouts via the attribute, e.g.

[RegexGenerator("abc", RegexOptions.None, 10_000)] // 10 seconds
private static partial Regex Abc();

If no timeout is specified, no timeout is applied, and the generator avoids spitting out the code necessary to check for timeouts, making it leaner and faster.

This is almost the same thing that RegexOptions.Compiled does. If there's no timeout, RegexOptions.Compiled will similarly avoid generating the code for the checks. The difference, however, is that whether there's a timeout is influenced not just by whether one is passed to the Regex ctor, but if one isn't passed to the Regex ctor, then the REGEX_DEFAULT_MATCH_TIMEOUT AppContext value is consulted and used.

So, we're currently not paying attention to REGEX_DEFAULT_MATCH_TIMEOUT in RegexGenerator. Options:

1. That's fine. If you want a timeout, specify a timeout. If you don't, don't.
2. Have the generator respect the REGEX_DEFAULT_MATCH_TIMEOUT at the time of build and from the build process. That's... weird. A developer would need to know what process is actually running the generator and figure out how to configure it.
3. If no timeout is supplied, change the generator to generate the timeout code as well as code to query REGEX_DEFAULT_MATCH_TIMEOUT on first use (e.g. in an assignment to a static readonly) and check for timeouts if it's not infinite. We might be able to structure this in a way where, with tiering, we could get the JIT to elide the timeout checks if no default timeout was supplied, though that would likely require opting-in to tiering with loops (which might become the default, anyway, as part of dynamic PGO enablement). Note that if an infinite timeout (-1) is explicitly specified, we are still free to avoid emitting timeout code, just as is the case with RegexOptions.Compiled.

cc: @GrabYourPitchforks

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

Issue Details

---

With the regex source generator merged in #59186, a developer can specify timeouts via the attribute, e.g.

[RegexGenerator("abc", RegexOptions.None, 10_000)] // 10 seconds
private static partial Regex Abc();

If no timeout is specified, no timeout is applied, and the generator avoids spitting out the code necessary to check for timeouts, making it leaner and faster.

This is almost the same thing that RegexOptions.Compiled does. If there's no timeout, RegexOptions.Compiled will similarly avoid generating the code for the checks. The difference, however, is that whether there's a timeout is influenced not just by whether one is passed to the Regex ctor, but if one isn't passed to the Regex ctor, then the REGEX_DEFAULT_MATCH_TIMEOUT AppContext value is consulted and used.

So, we're currently not paying attention to REGEX_DEFAULT_MATCH_TIMEOUT in RegexGenerator. Options:

1. That's fine. If you want a timeout, specify a timeout. If you don't, don't.
2. Have the generator respect the REGEX_DEFAULT_MATCH_TIMEOUT at the time of build and from the build process. That's... weird. A developer would need to know what process is actually running the generator and figure out how to configure it.
3. If no timeout is supplied, change the generator to generate the timeout code as well as code to query REGEX_DEFAULT_MATCH_TIMEOUT on first use (e.g. in an assignment to a static readonly) and check for timeouts if it's not infinite. We might be able to structure this in a way where, with tiering, we could get the JIT to elide the timeout checks if no default timeout was supplied, though that would likely require opting-in to tiering with loops (which might become the default, anyway, as part of dynamic PGO enablement). Note that if an infinite timeout (-1) is explicitly specified, we are still free to avoid emitting timeout code, just as is the case with RegexOptions.Compiled.

cc: @GrabYourPitchforks

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

[GSPP]

There could be an opt-in way to move the configuration for culture and timeout to execution time entirely. The codegen would then remain flexible to support any options. The options could be passed in using another overload and an options object. I understand that would entail a perf loss but it would be a rather clean design.

I wonder how much performance it would cost to insert timeout checks in the form if (timeoutEnabled) CheckTimeout();. That can't be too bad, right? It would be a perfectly predicted branch and a fairly small code size increase.

Another option would be to generate the code two times once with timeouts and once without. Obviously doubles code size.

[stephentoub]

move the configuration for culture and timeout to execution time entirely

That is one of the enumerated options.

That can't be too bad, right?

That's what used to exist. It is small but measurable, which is why we optimized it out. It's also why I discussed structuring things so the JIT can optimize out the checks.

Another option would be to generate the code two times once with timeouts and once without. Obviously doubles code size.

I didn't include this option as I don't think it should be considered. There can be a lot of code generated and it's not worth the duplication.