Allow GlobalizationMode.Invariant = false to be substituted by the trimmer.

[eerhardt]

This allows a Blazor WASM app to trim unused Invariant code when Invariant mode isn't enabled.

From my local measurements, this allows for a size savings of 3.7 KB .br compressed (10.7 KB uncompressed) in a default Blazor WASM app. An incomplete list of the public and internal members that can be trimmed when explicitly set (ApiReviewer wasn't able to show private members for some reason):

This will also require explicitly defaulting <InvariantGlobalization>false</InvariantGlobalization> in the Blazor WASM SDK, when it is not already set by the developer's project.

cc @stephentoub

[ghost]

Tagging subscribers to this area: @tarekgh, @safern, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This allows a Blazor WASM app to trim unused Invariant code when Invariant mode isn't enabled.

From my local measurements, this allows for a size savings of 3.7 KB .br compressed (10.7 KB uncompressed) in a default Blazor WASM app. An incomplete list of the public and internal members that can be trimmed when explicitly set (ApiReviewer wasn't able to show private members for some reason):

This will also require explicitly defaulting <InvariantGlobalization>false</InvariantGlobalization> in the Blazor WASM SDK, when it is not already set by the developer's project.

cc @stephentoub

Author:	eerhardt
Assignees:	-
Labels:	area-System.Globalization
Milestone:	-

[ghost]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar, @tannergooding
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This allows a Blazor WASM app to trim unused Invariant code when Invariant mode isn't enabled.

From my local measurements, this allows for a size savings of 3.7 KB .br compressed (10.7 KB uncompressed) in a default Blazor WASM app. An incomplete list of the public and internal members that can be trimmed when explicitly set (ApiReviewer wasn't able to show private members for some reason):

This will also require explicitly defaulting <InvariantGlobalization>false</InvariantGlobalization> in the Blazor WASM SDK, when it is not already set by the developer's project.

cc @stephentoub

Author:	eerhardt
Assignees:	-
Labels:	area-System.Globalization, size-reduction
Milestone:	-

[eerhardt]

FYI - @pranavkm @captainsafia. In order to benefit from this change, I'll need to make a corresponding change in the Blazor WASM SDK to default:

<PropertyGroup>
  <InvariantGlobalization Condition="'$(InvariantGlobalization)' == ''">false</InvariantGlobalization>

[stephentoub]

Are there other cases like this? I'm wondering whether we need more cases of handling both the true and false values, and if so, if there's a more general feature needed here, where the linker sees a line like:

<method signature="System.Boolean get_Invariant()" body="stub" feature="System.Globalization.Invariant" featurevalue="true" />

and automatically propagates featurevalue to value, or something like that.

[tannergooding]

I had proposed this in chat to @eerhardt the other day since it means we are potentially leaving behind bits of "unused" code that are statically known to only be true/false and therefore have no side effects.

dotnet/linker#1106 (comment) is a related example where just leaving the get_IsSupported() methoods can lead to larger downstream impact due to inheritance chains.

[eerhardt]

In all our existing feature switches, only 1 value will trim any substantial piece of code. For Invariant, I wasn't aware how much unnecessary code was still being rooted for the default setting. I thought only Invariant=true would trim a decent amount of code.

But I can imagine this being beneficial in other feature switches in the future.

cc @marek-safar @vitek-karas @sbomer - thoughts?

[tannergooding]

A related bit is that even needing to hardcode get_Invariant and others seems potentially unnecessary in certain cases

One would expect the linker could recognize the general pattern of static readonly bool s_name = AppContext.TryGetSwitch("", out bool result) ? result : default (or others) and that we wouldn't need to hardcode a table of substitutions outside of more complicated scenarios.
It, plus knowing that static readonly <primitive> name = value can't change after being set seem like generally good things to recognize as they can lead to later downstream optimizations and therefore trimming opportunities

[vitek-karas]

If it's common we could make syntax which would be able to substitute the value directly from the feature value. Note that even boolean features have 3 values: true, false, not-set. I assume the "not-set" would mean no substitution (unless the defaultfeaturevalue is used).

If this is the only place right now, it's probably not worth it though.

[eerhardt]

One would expect the linker could recognize the general pattern

That could work for other cases, but it doesn't work for this case:

runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs

Lines 10 to 38 in 8a52f1e

	internal static bool Invariant { get; } = GetGlobalizationInvariantMode();
	internal static bool UseNls => false;
	private static bool GetGlobalizationInvariantMode()
	{
	bool invariantEnabled = GetInvariantSwitchValue();
	if (!invariantEnabled)
	{
	if (TryGetAppLocalIcuSwitchValue(out string? icuSuffixAndVersion))
	{
	LoadAppLocalIcu(icuSuffixAndVersion);
	}
	else
	{
	int loaded = Interop.Globalization.LoadICU();
	if (loaded == 0 && !OperatingSystem.IsBrowser())
	{
	string message = "Couldn't find a valid ICU package installed on the system. " +
	"Set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support.";
	Environment.FailFast(message);
	}
	// fallback to Invariant mode if LoadICU failed (Browser).
	return loaded == 0;
	}
	}
	return invariantEnabled;
	}

[SamMonoRT]

@CoffeeFlux - for tracking

[captainsafia]

@eerhardt Sounds good. BTW -- the Blazor WASM SDK lives in the SDK repo now: https://github.com/dotnet/sdk/tree/master/src/BlazorWasmSdk

[marek-safar]

Not only Blazor but all size sensitive SDKs need to be updated. Please update iOS and Android as well.

@vitek-karas (real example of what we talked about a few months ago).

[eerhardt]

Not only Blazor but all size sensitive SDKs need to be updated. Please update iOS and Android as well.

Done:

• Default InvariantGlobalization in Blazor WASM sdk#15770
• Default InvariantGlobalization in Xamarin Android android#5605
• Default InvariantGlobalization in Xamarin IOS xamarin/xamarin-macios#10598