Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser] Trigger relink on EmccMaximumHeapSize change #105027

Merged
merged 11 commits into from
Jul 25, 2024
2 changes: 1 addition & 1 deletion eng/testing/linker/project.csproj.template
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@

<Target Name="RemoveInvariantGlobalization" BeforeTargets="_SetWasmBuildNativeDefaults" Condition="'$(TargetArchitecture)' == 'wasm'">
<ItemGroup>
<_BoolPropertiesThatTriggerRelinking Remove="InvariantGlobalization" />
<_PropertiesThatTriggerRelinking Remove="InvariantGlobalization" />
</ItemGroup>
</Target>

Expand Down
6 changes: 4 additions & 2 deletions src/mono/browser/browser.proj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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)" },
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
{ "identity": "EmccMaximumHeapSize", "defaultValueInRuntimePack": "$(EmccMaximumHeapSize)" }
]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasi/wasi.proj
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@
{
"items": {
"WasmOptConfigurationFlags": [@(WasmOptConfigurationFlags -> '%22%(Identity)%22', ',')],
"BoolPropertiesThatTriggerRelinking": [
"PropertiesThatTriggerRelinking": [
{ "identity": "InvariantTimezone", "defaultValueInRuntimePack": "$(InvariantTimezone)" },
{ "identity": "InvariantGlobalization", "defaultValueInRuntimePack": "$(InvariantGlobalization)" },
{ "identity": "WasmNativeStrip", "defaultValueInRuntimePack": "$(WasmNativeStrip)" },
Expand Down
7 changes: 7 additions & 0 deletions src/mono/wasm/Wasm.Build.Tests/Common/CommandResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 11 additions & 2 deletions src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/AppTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
26 changes: 14 additions & 12 deletions src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}
}
12 changes: 6 additions & 6 deletions src/mono/wasm/build/WasmApp.Common.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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) != ''">
Expand Down Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@
</ItemGroup>

<ItemGroup>
<BlazorWebAssemblyLazyLoad Include="Json$(LazyAssemblyExtension)" />
<BlazorWebAssemblyLazyLoad Include="Json$(WasmAssemblyExtension)" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ function testOutput(msg) {

function countChars(str) {
const length = str.length;
testOutput(`JS received str of ${length} length`);
return length;
}

Expand Down
Loading