Enable MetricsSupport by default for non-Release builds #9928
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the heads-up, I've changed our logic to be identical to this (dotnet/macios#22389). |
|
@jfversluis sorry silly question, but what does the MetricsSupport actually do? Do you have any docs I can look at? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
because in Debug config builds, the We'll need to update the test to not assert the non-existence of this field in Debug builds. |
Update the assertion accordingly.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rolfbjarne
added a commit
to dotnet/macios
that referenced
this pull request
Mar 19, 2025
… builds (#22389) This saves ~191kb / 0.3% of app size on a minimal hello world iOS app. Android is already disabling this by default since .NET 8, but will be for non-Release builds only in .NET 10 (dotnet/android#9928). Ref: * dotnet/runtime#111801 (comment) * https://github.com/dotnet/android/blob/599bd6c8fb3ffb1a0e09d06fb2b1fd69b8a57101/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets#L128 * dotnet/android#9928
|
Draft commit message: [Xamarin.Android.Build.Tasks] $(MetricsSupport)=false in "Release" (#9928)
Context: eb6397d4a9dcef69e1d012fd52fbb9688557b0b4
Context: 200f843c819a634b3c7921738de4d726c8e675c7
Context: https://github.com/dotnet/runtime/issues/89880
Context: https://github.com/dotnet/aspire/issues/4684
Context: https://github.com/dotnet/maui/pull/24365
Context: https://github.com/jfversluis/MauiAspire
.NET 8 introduced use of System.Diagnostics.Metrics into `HttpClient`,
which immediately slowed down .NET for Android app startup times
(dotnet/runtime#89880). This was "fixed" by *disabling* use of Metrics
within .NET for Android apps *by default* (eb6397d4, 200f843c).
Support for Metrics could be re-enabled by setting the
[`$(MetricsSupport)` MSBuild property][0].
.NET Aspire also uses System.Diagnostics.Metrics, and we'd like to
allow Metrics to be available by default in that environment; see also
dotnet/aspire#4684 and dotnet/maui#24365.
Update the `$(MetricsSupport)` MSBuild property to be false by default
only in "Release" configuration builds.
[0]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options@jfversluis: is there anything useful we should link to or mention wrt .NET MAUI, .NET Aspire, and System.Diagnostics.Metrics? |
jonpryor
reviewed
Mar 19, 2025
| @@ -125,7 +125,7 @@ | |||
| <JsonSerializerIsReflectionEnabledByDefault Condition="'$(JsonSerializerIsReflectionEnabledByDefault)' == '' and '$(TrimMode)' == 'partial'">true</JsonSerializerIsReflectionEnabledByDefault> | |||
| <!-- Set to disable throwing behavior, see: https://github.com/dotnet/runtime/issues/109724 --> | |||
| <_DefaultValueAttributeSupport Condition="'$(_DefaultValueAttributeSupport)' == '' and '$(TrimMode)' == 'partial'">true</_DefaultValueAttributeSupport> | |||
| <MetricsSupport Condition="'$(MetricsSupport)' == ''">false</MetricsSupport> | |||
| <MetricsSupport Condition="'$(MetricsSupport)' == '' and '$(Configuration)' == 'Release'">false</MetricsSupport> | |||
There was a problem hiding this comment.
@dellis1972: What are your thoughts on using $(Configuration) in this manner? I think historically we've tried to avoid this, because it's not uncommon for people to "invent" new configuration names. Is there some other property we should use instead, perhaps '$(DebugSymbols)' == 'True'?
|
@jfversluis: I imagine dotnet/aspire#4684 is a relevant link? |
|
wrt merging this, I'm not sold on using |
Use `'$(Optimize)' == 'true'`
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rolfbjarne
added a commit
to dotnet/macios
that referenced
this pull request
Mar 21, 2025
Disable Metrics support when `Optimize=true` (instead of when `Release=true`) to avoid depending on a configuration. This makes our logic identical to Android's logic: dotnet/android#9928.
rolfbjarne
added a commit
to dotnet/macios
that referenced
this pull request
Mar 25, 2025
Disable Metrics support when `Optimize=true` (instead of when `Release=true`) to avoid depending on a configuration. This makes our logic identical to Android's logic: dotnet/android#9928.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Primarily to light up the metrics when using .NET MAUI apps with .NET Aspire, but this will emit metrics for potential other purposes as well.
With this change the
MetricsSupportproperty will only be set tofalsewhen the property has no value and the build is not aReleasebuild.That means, if the property is not explicitly set in the .NET MAUI project (or .NET for Android project) and the build is not a
Releasebuild, the property will be set totrue(the default value) and emit metrics.cc: @rolfbjarne I was informed you were going to disable this for iOS, so it might be good to keep this in mind as well for that!