[browser] Trigger relink on EmccMaximumHeapSize change (#105027)

* Edit test + trigger relink. * Remove logging to speed up the test + decrease loop runs to prevent "Browser has been disconnected" error. * Feedback - properties are not bool-only anymore. * Fix: workload needed when heap size set. --------- Co-authored-by: Larry Ewing <[email protected]>
dotnet · Jul 26, 2024 · f6bca30 · f6bca30
1 parent 7942b5b
commit f6bca30
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 29 deletions.
diff --git a/eng/testing/linker/project.csproj.template b/eng/testing/linker/project.csproj.template
@@ -77,7 +77,7 @@
 
   <Target Name="RemoveInvariantGlobalization" BeforeTargets="_SetWasmBuildNativeDefaults" Condition="'$(TargetArchitecture)' == 'wasm'">
     <ItemGroup>
-      <_BoolPropertiesThatTriggerRelinking Remove="InvariantGlobalization" />
+      <_PropertiesThatTriggerRelinking Remove="InvariantGlobalization" />
     </ItemGroup>
   </Target>
 

diff --git a/src/mono/browser/browser.proj b/src/mono/browser/browser.proj
@@ -29,6 +29,7 @@
     <WasmSingleFileBundle Condition="'$(WasmSingleFileBundle)' == ''">false</WasmSingleFileBundle>
     <WasmEnableSIMD Condition="'$(WasmEnableSIMD)' == ''">true</WasmEnableSIMD>
     <WasmEnableExceptionHandling Condition="'$(WasmEnableExceptionHandling)' == ''">true</WasmEnableExceptionHandling>
+    <EmccMaximumHeapSize Condition="'$(EmccMaximumHeapSize)' == ''">2147483648</EmccMaximumHeapSize>
     <WasmEnableJsInteropByValue Condition="'$(WasmEnableJsInteropByValue)' == '' and '$(WasmEnableThreads)' == 'true'">true</WasmEnableJsInteropByValue>
     <WasmEnableJsInteropByValue Condition="'$(WasmEnableJsInteropByValue)' == ''">false</WasmEnableJsInteropByValue>
     <FilterSystemTimeZones Condition="'$(FilterSystemTimeZones)' == ''">false</FilterSystemTimeZones>
@@ -334,13 +335,14 @@
     "WasmOptConfigurationFlags": [@(WasmOptConfigurationFlags -> '%22%(Identity)%22', ',')],
     "EmccDefaultExportedFunctions": [@(EmccExportedFunction -> '%22%(Identity)%22', ',')],
     "EmccDefaultExportedRuntimeMethods": [@(EmccExportedRuntimeMethod -> '%22%(Identity)%22', ',')],
-    "BoolPropertiesThatTriggerRelinking": [
+    "PropertiesThatTriggerRelinking": [
       { "identity": "InvariantTimezone",            "defaultValueInRuntimePack": "$(InvariantTimezone)" },
       { "identity": "InvariantGlobalization",       "defaultValueInRuntimePack": "$(InvariantGlobalization)" },
       { "identity": "WasmNativeStrip",              "defaultValueInRuntimePack": "$(WasmNativeStrip)" },
       { "identity": "WasmSingleFileBundle",         "defaultValueInRuntimePack": "$(WasmSingleFileBundle)" },
       { "identity": "WasmEnableSIMD",               "defaultValueInRuntimePack": "$(WasmEnableSIMD)" },
-      { "identity": "WasmEnableExceptionHandling",  "defaultValueInRuntimePack": "$(WasmEnableExceptionHandling)" }
+      { "identity": "WasmEnableExceptionHandling",  "defaultValueInRuntimePack": "$(WasmEnableExceptionHandling)" },
+      { "identity": "EmccMaximumHeapSize",          "defaultValueInRuntimePack": "$(EmccMaximumHeapSize)" }
     ]
   }
 }

diff --git a/.../nuget/Microsoft.NET.Workload.Mono.Toolchain.Current.Manifest/WorkloadManifest.targets.in b/.../nuget/Microsoft.NET.Workload.Mono.Toolchain.Current.Manifest/WorkloadManifest.targets.in
@@ -58,7 +58,8 @@
         '$(RunAOTCompilation)' == 'true' or
         '$(WasmBuildNative)' == 'true' or
         '$(WasmGenerateAppBundle)' == 'true' or
-        '$(_UsingBlazorOrWasmSdk)' != 'true'" >true</_WasmNativeWorkloadNeeded>
+        '$(_UsingBlazorOrWasmSdk)' != 'true' or
+        '$(EmccMaximumHeapSize)' != '' " >true</_WasmNativeWorkloadNeeded>
 
     <UsingBrowserRuntimeWorkload Condition="'$(_BrowserWorkloadNotSupportedForTFM)' == 'true'">false</UsingBrowserRuntimeWorkload>
     <UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == '' and '$(_WasmNativeWorkloadNeeded)' == 'true'">true</UsingBrowserRuntimeWorkload>

diff --git a/src/mono/wasi/wasi.proj b/src/mono/wasi/wasi.proj
@@ -158,7 +158,7 @@
 {
   "items": {
     "WasmOptConfigurationFlags": [@(WasmOptConfigurationFlags -> '%22%(Identity)%22', ',')],
-    "BoolPropertiesThatTriggerRelinking": [
+    "PropertiesThatTriggerRelinking": [
       { "identity": "InvariantTimezone",      "defaultValueInRuntimePack": "$(InvariantTimezone)" },
       { "identity": "InvariantGlobalization", "defaultValueInRuntimePack": "$(InvariantGlobalization)" },
       { "identity": "WasmNativeStrip",        "defaultValueInRuntimePack": "$(WasmNativeStrip)" },

diff --git a/src/mono/wasm/Wasm.Build.Tests/Common/CommandResult.cs b/src/mono/wasm/Wasm.Build.Tests/Common/CommandResult.cs
@@ -27,6 +27,13 @@ public CommandResult(ProcessStartInfo startInfo, int exitCode, string output)
         public CommandResult EnsureSuccessful(string messagePrefix = "", bool suppressOutput = false)
             => EnsureExitCode(0, messagePrefix, suppressOutput);
 
+        public CommandResult EnsureFailed(string messagePrefix = "", bool suppressOutput = false)
+        {
+            if (ExitCode == 0)
+                throw new XunitException($"{messagePrefix} Expected non-zero exit code but got 0: {StartInfo.FileName} {StartInfo.Arguments}");
+            return this;
+        }
+
         public CommandResult EnsureExitCode(int expectedExitCode = 0, string messagePrefix = "", bool suppressOutput = false)
         {
             if (ExitCode != expectedExitCode)

diff --git a/src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/AppTestBase.cs b/src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/AppTestBase.cs
@@ -59,15 +59,24 @@ protected void BuildProject(
         string? binFrameworkDir = null,
         RuntimeVariant runtimeType = RuntimeVariant.SingleThreaded,
         bool assertAppBundle = true,
+        bool expectSuccess = true,
         params string[] extraArgs)
     {
         (CommandResult result, _) = BlazorBuild(new BlazorBuildOptions(
             Id: Id,
             Config: configuration,
             BinFrameworkDir: binFrameworkDir,
             RuntimeType: runtimeType,
-            AssertAppBundle: assertAppBundle), extraArgs);
-        result.EnsureSuccessful();
+            AssertAppBundle: assertAppBundle,
+            ExpectSuccess: expectSuccess), extraArgs);
+        if (expectSuccess)
+        {
+            result.EnsureSuccessful();
+        }
+        else
+        {
+            result.EnsureFailed();
+        }
     }
 
     protected void PublishProject(

diff --git a/src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/MemoryTests.cs b/src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/MemoryTests.cs
@@ -19,20 +19,22 @@ public MemoryTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buil
     {
     }
 
-    [Theory]
-    [InlineData("Release", true)]
-    [InlineData("Release", false)]
-    [ActiveIssue("https://github.com/dotnet/runtime/issues/105283")]
-    public async Task AllocateLargeHeapThenRepeatedlyInterop(string config, bool buildNative)
+    // ActiveIssue: https://github.com/dotnet/runtime/issues/104618
+    [Fact, TestCategory("no-workload")]
+    public async Task AllocateLargeHeapThenRepeatedlyInterop_NoWorkload() =>
+        await AllocateLargeHeapThenRepeatedlyInterop();
+
+    [Fact]
+    public async Task AllocateLargeHeapThenRepeatedlyInterop()
     {
-        // native build triggers passing value form EmccMaximumHeapSize to MAXIMUM_MEMORY that is set in emscripten
-        // in non-native build EmccMaximumHeapSize does not have an effect, so the test will fail with "out of memory"
+        string config = "Release";
         CopyTestAsset("WasmBasicTestApp", "MemoryTests", "App");
-        string extraArgs = $"-p:EmccMaximumHeapSize=4294901760 -p:WasmBuildNative={buildNative}";
-        BuildProject(config, assertAppBundle: false, extraArgs: extraArgs);
+        string extraArgs = BuildTestBase.IsUsingWorkloads ? "-p:EmccMaximumHeapSize=4294901760" : "-p:EmccMaximumHeapSize=4294901760";
+        BuildProject(config, assertAppBundle: false, extraArgs: extraArgs, expectSuccess: BuildTestBase.IsUsingWorkloads);
 
-        var result = await RunSdkStyleAppForBuild(new (Configuration: config, TestScenario: "AllocateLargeHeapThenInterop", ExpectedExitCode: buildNative ? 0 : 1));
-        if (!buildNative)
-            Assert.Contains(result.TestOutput, item => item.Contains("Exception System.OutOfMemoryException: Out of memory"));
+        if (BuildTestBase.IsUsingWorkloads)
+        {
+            await RunSdkStyleAppForBuild(new (Configuration: config, TestScenario: "AllocateLargeHeapThenInterop"));
+        }
     }
 }
diff --git a/src/mono/wasm/build/WasmApp.Common.targets b/src/mono/wasm/build/WasmApp.Common.targets
@@ -393,7 +393,7 @@
       <WasmOptConfigurationFlags ParameterType="Microsoft.Build.Framework.ITaskItem[]" Required="false" Output="true" />
       <EmccDefaultExportedFunctions ParameterType="Microsoft.Build.Framework.ITaskItem[]" Required="false" Output="true" />
       <EmccDefaultExportedRuntimeMethods ParameterType="Microsoft.Build.Framework.ITaskItem[]" Required="false" Output="true" />
-      <BoolPropertiesThatTriggerRelinking ParameterType="Microsoft.Build.Framework.ITaskItem[]" Required="false" Output="true" />
+      <PropertiesThatTriggerRelinking ParameterType="Microsoft.Build.Framework.ITaskItem[]" Required="false" Output="true" />
     </ParameterGroup>
   </UsingTask>
 
@@ -405,7 +405,7 @@
 
       <!-- shared by browser/wasi -->
       <Output TaskParameter="WasmOptConfigurationFlags" ItemName="_DefaulWasmOptConfigurationFlags" />
-      <Output TaskParameter="BoolPropertiesThatTriggerRelinking" ItemName="_BoolPropertiesThatTriggerRelinking" />
+      <Output TaskParameter="PropertiesThatTriggerRelinking" ItemName="_PropertiesThatTriggerRelinking" />
     </ReadWasmProps>
 
     <CreateProperty Value="%(_EmccPropItems.Value)" Condition="%(_EmccPropItems.Identity) != ''">
@@ -511,15 +511,15 @@
            Text="$(_ToolchainMissingErrorMessage) SDK is required for AOT'ing assemblies." />
 
     <ItemGroup>
-      <_ChangedBoolPropertiesThatTriggerRelinking Include="%(_BoolPropertiesThatTriggerRelinking.Identity)" Condition="'$(%(_BoolPropertiesThatTriggerRelinking.Identity))' != '' and
-                                                                                                            '$(%(_BoolPropertiesThatTriggerRelinking.Identity))' != '%(_BoolPropertiesThatTriggerRelinking.DefaultValueInRuntimePack)'" />
+      <_ChangedPropertiesThatTriggerRelinking Include="%(_PropertiesThatTriggerRelinking.Identity)" Condition="'$(%(_PropertiesThatTriggerRelinking.Identity))' != '' and
+                                                                                                            '$(%(_PropertiesThatTriggerRelinking.Identity))' != '%(_PropertiesThatTriggerRelinking.DefaultValueInRuntimePack)'" />
     </ItemGroup>
     <PropertyGroup>
-      <_WasmBuildNativeRequired Condition="@(_ChangedBoolPropertiesThatTriggerRelinking->Count()) > 0">true</_WasmBuildNativeRequired>
+      <_WasmBuildNativeRequired Condition="@(_ChangedPropertiesThatTriggerRelinking->Count()) > 0">true</_WasmBuildNativeRequired>
     </PropertyGroup>
 
     <Error Condition="'$(WasmBuildNative)' == 'false' and '$(_WasmBuildNativeRequired)' == 'true'"
-           Text="WasmBuildNative is required because %(_ChangedBoolPropertiesThatTriggerRelinking.Identity)=$(%(_ChangedBoolPropertiesThatTriggerRelinking.Identity)), but WasmBuildNative is already set to 'false'." />
+           Text="WasmBuildNative is required because %(_ChangedPropertiesThatTriggerRelinking.Identity)=$(%(_ChangedPropertiesThatTriggerRelinking.Identity)), but WasmBuildNative is already set to 'false'." />
 
     <PropertyGroup>
       <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(_WasmBuildNativeRequired)' == 'true'">true</WasmBuildNative>

diff --git a/src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs b/src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs
@@ -7,7 +7,7 @@
 using System.Text;
 using System.Runtime.InteropServices.JavaScript;
 
-public partial class MemoryTest // ?test=AllocateLargeHeapThenInterop
+public partial class MemoryTest
 {
     [JSImport("countChars", "main.js")]
     internal static partial int CountChars(string testArray);
@@ -37,7 +37,7 @@ internal static void Run()
         string randomString = GenerateRandomString(1000);
         try
         {
-            for (int i = 0; i < 10000; i++)
+            for (int i = 0; i < 1000; i++)
             {
                 int count = CountChars(randomString);
                 if (count != randomString.Length)

diff --git a/src/mono/wasm/testassets/WasmBasicTestApp/App/WasmBasicTestApp.csproj b/src/mono/wasm/testassets/WasmBasicTestApp/App/WasmBasicTestApp.csproj
@@ -19,6 +19,6 @@
   </ItemGroup>
 
   <ItemGroup>
-    <BlazorWebAssemblyLazyLoad Include="Json$(LazyAssemblyExtension)" />
+    <BlazorWebAssemblyLazyLoad Include="Json$(WasmAssemblyExtension)" />
   </ItemGroup>
 </Project>
diff --git a/src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js b/src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js
@@ -16,7 +16,6 @@ function testOutput(msg) {
 
 function countChars(str) {
     const length = str.length;
-    testOutput(`JS received str of ${length} length`);
     return length;
 }