RegexOptions.Compiled impacts Android startup #71007

jonathanpeppers · 2022-06-20T16:31:04Z

Description

Reviewing a customer issue, we noticed usage of RegexOptions.Compiled impacts startup quite a bit:

This was coming from the AppCenter SDK in the customer's app:

https://github.com/microsoft/appcenter-sdk-dotnet/blob/b19ec99e16e554eb1382342b2852b16f7d8f0084/SDK/AppCenter/Microsoft.AppCenter.Shared/AppCenter.cs#L26

This shows up as a regression from Xamarin.Android -> .NET 6, because RegexOptions.Compiled was not implemented at all in mono/mono.

The question here, is how do we get the previous behavior from mono/mono? Is there a way to toggle RuntimeFeature.IsDynamicCodeCompiled?

runtime/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

Line 105 in 807d091

    
           if (RuntimeFeature.IsDynamicCodeCompiled && (options & RegexOptions.Compiled) != 0)

If there is a way to toggle RuntimeFeature.IsDynamicCodeCompiled, should we consider turning on this flag by default on Android? I've found System.Reflection.Emit to be very impactful to startup.

Reproduction Steps

dotnet new android
Open MainActivity.cs and put _ = new Regex(@"([^;=]+)=([^;]+)(?:;\s*)?", RegexOptions.Compiled); in the OnCreate() method.
Build/run with dotnet build -c Release -t:Run

Expected behavior

RegexOptions.Compiled doesn't have a significant impact to Android startup. (Or we have a feature flag to turn it off)

Actual behavior

RegexOptions.Compiled has a significant impact to Android startup.

Regression?

From a customer's perspective they might say this is startup performance regression from Xamarin.Android to .NET 6.

Known Workarounds

Don't use RegexOptions.Compiled, use Normal.

Configuration

I tested this with the .NET 6.0.400-preview.22301.10 SDK, and the SR 1 version of .NET MAUI.

Other information

dotnet trace output: regexoptions.compiled.zip

The text was updated successfully, but these errors were encountered:

ghost · 2022-06-20T16:31:16Z

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

Issue Details

Description

Reviewing a customer issue, we noticed usage of RegexOptions.Compiled impacts startup quite a bit:

This was coming from the AppCenter SDK:

https://github.com/microsoft/appcenter-sdk-dotnet/blob/b19ec99e16e554eb1382342b2852b16f7d8f0084/SDK/AppCenter/Microsoft.AppCenter.Shared/AppCenter.cs#L26

This shows up as a regression from Xamarin.Android -> .NET 6, because RegexOptions.Compiled was not implemented at all in mono/mono.

The question here, is how do we get the previous behavior from mono/mono? Is there a way to toggle RuntimeFeature.IsDynamicCodeCompiled?

runtime/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

Line 105 in 807d091

    
           if (RuntimeFeature.IsDynamicCodeCompiled && (options & RegexOptions.Compiled) != 0)

If there is a way to toggle RuntimeFeature.IsDynamicCodeCompiled, should we consider turning on this flag by default on Android? I've found System.Reflection.Emit to be very impactful to startup.

Reproduction Steps

dotnet new android
Open MainActivity.cs and put _ = new Regex(@"([^;=]+)=([^;]+)(?:;\s*)?", RegexOptions.Compiled); in the OnCreate() method.
Build/run with dotnet build -c Release -t:Run

Expected behavior

RegexOptions.Compiled don'ts have a significant impact to Android startup. (Or we have a feature flag to turn it off)

Actual behavior

RegexOptions.Compiled has a significant impact to Android startup.

Regression?

From a customer's perspective they might say this is startup performance regression from Xamarin.Android to .NET 6.

Known Workarounds

Don't use RegexOptions.Compiled, use Normal.

Configuration

I tested this with the .NET 6.0.400-preview.22301.10 SDK, and the SR 1 version of .NET MAUI.

Other information

dotnet trace output: regexoptions.compiled.zip

Author:	jonathanpeppers
Assignees:	-
Labels:	`area-System.Text.RegularExpressions`
Milestone:	-

stephentoub · 2022-06-20T16:47:14Z

RegexOptions.Compiled is a tradeoff between startup overhead and throughput: it adds overhead to constructing the Regex in exchange for making regexes execute faster. Are you seeing this construction cost increase but throughput not also increase? If they both go up, then the opt-in flag is doing what it's supposed to do, and if a customer doesn't want to tradeoff that construction overhead for throughput, they shouldn't be setting Compiled to opt-in. (And if they want both, they should consider using the regex source generator in .NET 7 instead; this will give them both the construction perf and the throughput perf, at the expense of a bit larger IL size.)

jonathanpeppers · 2022-06-20T17:27:35Z

In this case, the customer's use of RegexOptions.Compiled was coming from a NuGet owned by Microsoft. I was curious if there is some way to opt out of RuntimeFeature.IsDynamicCodeCompiled? Or some way to get back to the behavior they had from Xamarin.Android (mono/mono)?

danmoseley · 2022-06-20T17:47:42Z

Just curious, why do they not pass it when targeting .NET Standard?

#if NETSTANDARD
        static readonly Regex _secretsRegex = new Regex(SecretsPattern);
#else
        static readonly Regex _secretsRegex = new Regex(SecretsPattern, RegexOptions.Compiled);
#endif

jonathanpeppers · 2022-06-20T18:58:30Z

If you look at blame, it doesn't actually give much detail:

microsoft/appcenter-sdk-dotnet@70f7944

I'm not sure what issue they're trying to solve here either. It seems like the old code would have worked in Xamarin.Android in 2019 to me?

danmoseley · 2022-06-20T19:56:39Z

My guess is that they thought it was still broken. I think the to-do here is to just pass on Stephen's guidance. Startup time, throughput, on disk size - pick two!

If there is a way to toggle RuntimeFeature.IsDynamicCodeCompiled, should we consider turning on this flag by default on Android? I've found System.Reflection.Emit to be very impactful to startup.

Question for @steveisok I think.

jonathanpeppers · 2022-06-20T20:06:38Z

On Android, we would pretty much never want throughput for anything. So that is why the default setting might be interesting.

ghost · 2022-06-20T22:23:46Z

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Reviewing a customer issue, we noticed usage of RegexOptions.Compiled impacts startup quite a bit:

This was coming from the AppCenter SDK in the customer's app:

https://github.com/microsoft/appcenter-sdk-dotnet/blob/b19ec99e16e554eb1382342b2852b16f7d8f0084/SDK/AppCenter/Microsoft.AppCenter.Shared/AppCenter.cs#L26

This shows up as a regression from Xamarin.Android -> .NET 6, because RegexOptions.Compiled was not implemented at all in mono/mono.

The question here, is how do we get the previous behavior from mono/mono? Is there a way to toggle RuntimeFeature.IsDynamicCodeCompiled?

runtime/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

Line 105 in 807d091

    
           if (RuntimeFeature.IsDynamicCodeCompiled && (options & RegexOptions.Compiled) != 0)

If there is a way to toggle RuntimeFeature.IsDynamicCodeCompiled, should we consider turning on this flag by default on Android? I've found System.Reflection.Emit to be very impactful to startup.

Reproduction Steps

dotnet new android
Open MainActivity.cs and put _ = new Regex(@"([^;=]+)=([^;]+)(?:;\s*)?", RegexOptions.Compiled); in the OnCreate() method.
Build/run with dotnet build -c Release -t:Run

Expected behavior

RegexOptions.Compiled doesn't have a significant impact to Android startup. (Or we have a feature flag to turn it off)

Actual behavior

RegexOptions.Compiled has a significant impact to Android startup.

Regression?

From a customer's perspective they might say this is startup performance regression from Xamarin.Android to .NET 6.

Known Workarounds

Don't use RegexOptions.Compiled, use Normal.

Configuration

I tested this with the .NET 6.0.400-preview.22301.10 SDK, and the SR 1 version of .NET MAUI.

Other information

dotnet trace output: regexoptions.compiled.zip

Author:	jonathanpeppers
Assignees:	-
Labels:	`area-System.Text.RegularExpressions`, `os-android`, `untriaged`
Milestone:	-

steveisok · 2022-06-20T23:13:16Z

If we were to do anything, I think it would have to be through a runtime feature switch. @jonathanpeppers, how significant are we talking vs what was in mono/mono?

teo-tsirpanis · 2022-06-21T09:14:32Z

Just curious, why do they not pass it when targeting .NET Standard?

@danmoseley they are targeting .NET Standard 1.0 only, which does not have Compiled.

https://github.com/microsoft/appcenter-sdk-dotnet/blob/b19ec99e16e554eb1382342b2852b16f7d8f0084/SDK/AppCenter/Microsoft.AppCenter/Microsoft.AppCenter.csproj#L6

akoeplinger · 2022-06-21T12:56:35Z

If you look at blame, it doesn't actually give much detail:

microsoft/appcenter-sdk-dotnet@70f7944

I'm not sure what issue they're trying to solve here either. It seems like the old code would have worked in Xamarin.Android in 2019 to me?

If you look at the PR from the commit and the linked AzDO bug you'll eventually find out that this was done to address microsoft/appcenter-sdk-dotnet#1247. I think because we didn't have the string.Split() overload which takes a single char in mono/mono.

I don't think we should do anything here, the feature works as expected. AppCenter should either not pass in RegexOptions.Compiled on mobile platforms or multi-target to net6.0 and switch back to the string.Split() logic there.

jonathanpeppers · 2022-06-23T16:32:49Z

We discussed this some, and we think:

Don't use RegexOptions.Compile on mobile, unless you want it for throughput
We should be careful about RuntimeFeature.IsDynamicCodeCompiled and its use for changes like: Add IL Emit support for MethodInfo.Invoke() and friends #69575

If we implement the same System.Reflection changes for Mono, we should test the impact on Android startup. Let me know if I can help with that.

We can close this for now, thanks.

danmoseley · 2022-06-23T17:44:22Z

Don't use RegexOptions.Compile on mobile, unless you want it for throughput

Longer term it would be interesting to develop some guidance around use of the regex source generator on mobile. I guess that will emerge as folks use it.

jonathanpeppers · 2022-10-12T19:19:30Z

It appears the new Regex APIs save about ~93.5ms of startup time on a Pixel 5 in a "hello world" Android app:

https://github.com/jonathanpeppers/AndroidRegex

I'm working on a blog post that will mention this in detail.

stephentoub · 2022-10-12T20:06:19Z

It appears the new Regex APIs save about ~93.5ms of startup time on a Pixel 5 in a "hello world" Android app:

Nice!

danmoseley · 2022-10-12T20:31:35Z

@jonathanpeppers I see the flame diagram there for "before", I'm curious what the flame diagram looks like "after" (if you collected it)

jonathanpeppers · 2022-10-12T20:57:24Z

Ah yes, so MainActivity..cctor is now so fast it is completely gone from the trace.

I added the .speedscope/.nettrace files and more screenshots: https://github.com/jonathanpeppers/AndroidRegex

danmoseley · 2022-10-12T21:06:00Z

If I read right, the ctor was 174ms and IsMatch another 49ms. Now the ctor is negligible (because source gen) and IsMatch is only 6ms (because it is smarter about finding there is no possible start position, apparently)

So I think I'd expect an improvement of over 200ms, not 93.5 ms?
Also I'm curious what is causing the CompileMethod in the newer IsMatch.

jonathanpeppers · 2022-10-12T21:29:33Z

My general observation, is these timings are a bit slower when traced -- especially on a mobile device running Mono. Maybe @lateralusX knows if there is some amount of overhead?

I normally test with & without tracing to double-check things.

jonathanpeppers · 2022-10-12T21:31:44Z

Also I'm curious what is causing the CompileMethod in the newer IsMatch.

RuntimeHelpers.CompileMethod() is a placeholder for time spent in the runtime, so it's likely JIT'ing this method, but it could also be loading additional assemblies, etc.

lateralusX · 2022-10-13T07:55:51Z

My general observation, is these timings are a bit slower when traced -- especially on a mobile device running Mono. Maybe @lateralusX knows if there is some amount of overhead?

I normally test with & without tracing to double-check things.

If you run sampling it will impact process since part of sampling includes a STW (stop the world for every sample) so it will affect timers running longer than the sample interval (1ms be default). If you don't run with sampling, the impact should be minimal. It should also be mentioned that the elapsed time in speed scope is a "calculated metric", but sampling should mainly be used in relation to the total number of samples (percent, flam graphs etc) to spot methods with most samples, that is how tools like perfview visualize sample data. If you want to measure wall clock times, you should do so by add specific trace events, use custom timers or in cases there is support for low level runtime profilers use build in method enter/leave support.

lateralusX · 2022-10-13T07:57:59Z

Also I'm curious what is causing the CompileMethod in the newer IsMatch.

RuntimeHelpers.CompileMethod() is a placeholder for time spent in the runtime, so it's likely JIT'ing this method, but it could also be loading additional assemblies, etc.

Jupp, CompileMethod is a place holder when runtime needs to JIT compile a method. Part of JIT compile a method might results in the need to load additional types (assemblies) and potential do some other runtime work, but CompileMethod gets triggered where there is a need to JIT or load an AOT:ed method. If you run tracing with JIT/Loader events you will find what method gets JIT:ed in those events and if any additional assemblies/types needs to be loaded as part of the process.

dotnet-issue-labeler bot added the area-System.Text.RegularExpressions label Jun 20, 2022

ghost added the untriaged New issue has not been triaged by the area owner label Jun 20, 2022

jonathanpeppers moved this to Todo in .NET 7 MAUI Performance Jun 20, 2022

jonathanpeppers added this to .NET 7 MAUI Performance Jun 20, 2022

danmoseley added the os-android label Jun 20, 2022

jonathanpeppers closed this as completed Jun 23, 2022

Repository owner moved this from Todo: Performance to Done: Performance in .NET 7 MAUI Performance Jun 23, 2022

ghost removed the untriaged New issue has not been triaged by the area owner label Jun 23, 2022

ghost locked as resolved and limited conversation to collaborators Jul 23, 2022

jonathanpeppers moved this from Done: Performance to Done & Blogged in .NET 7 MAUI Performance Oct 12, 2022

RegexOptions.Compiled impacts Android startup #71007

RegexOptions.Compiled impacts Android startup #71007

Comments

jonathanpeppers commented Jun 20, 2022 • edited Loading

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

ghost commented Jun 20, 2022

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

stephentoub commented Jun 20, 2022 • edited Loading

jonathanpeppers commented Jun 20, 2022

danmoseley commented Jun 20, 2022

jonathanpeppers commented Jun 20, 2022

danmoseley commented Jun 20, 2022 • edited Loading

jonathanpeppers commented Jun 20, 2022

ghost commented Jun 20, 2022

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

steveisok commented Jun 20, 2022

teo-tsirpanis commented Jun 21, 2022 • edited Loading

akoeplinger commented Jun 21, 2022

jonathanpeppers commented Jun 23, 2022

danmoseley commented Jun 23, 2022

jonathanpeppers commented Oct 12, 2022 • edited Loading

stephentoub commented Oct 12, 2022

danmoseley commented Oct 12, 2022

jonathanpeppers commented Oct 12, 2022

danmoseley commented Oct 12, 2022 • edited Loading

jonathanpeppers commented Oct 12, 2022

jonathanpeppers commented Oct 12, 2022

lateralusX commented Oct 13, 2022 • edited Loading

lateralusX commented Oct 13, 2022 • edited Loading

jonathanpeppers commented Jun 20, 2022 •

edited

Loading

stephentoub commented Jun 20, 2022 •

edited

Loading

danmoseley commented Jun 20, 2022 •

edited

Loading

teo-tsirpanis commented Jun 21, 2022 •

edited

Loading

jonathanpeppers commented Oct 12, 2022 •

edited

Loading

danmoseley commented Oct 12, 2022 •

edited

Loading

lateralusX commented Oct 13, 2022 •

edited

Loading

lateralusX commented Oct 13, 2022 •

edited

Loading