-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[browser] tests and fixes for heap larger than 2GB #104662
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
23a491c
Add wbt
ilonatommy 40c7993
Test all cases, including workaround.
ilonatommy f86cdfe
Remove workaround case from wbt.
ilonatommy 6144bdc
Merge branch 'main' into fix-104618
ilonatommy d9a1fa6
Clean up temporary changes.
ilonatommy ba4faf2
Fix typo
ilonatommy 3237684
Merge branch 'main' into fix-104618
ilonatommy bfd3692
Feedback - failing with "MONO_WASM: Assert failed: ES6 module 籨Ȍ ( h …
ilonatommy ddc89f4
Merge branch 'fix-104618' of https://github.com/ilonatommy/runtime in…
ilonatommy 5138eb4
Missing: run the new wbt on CI.
ilonatommy 4ab6e9d
Correct misconception about the feedback.
ilonatommy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
37 changes: 37 additions & 0 deletions
37
src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/MemoryTests.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using Xunit.Abstractions; | ||
| using Xunit; | ||
|
|
||
| #nullable enable | ||
|
|
||
| namespace Wasm.Build.Tests.TestAppScenarios; | ||
|
|
||
| public class MemoryTests : AppTestBase | ||
| { | ||
| public MemoryTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) | ||
| : base(output, buildContext) | ||
| { | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("Release", true)] | ||
| [InlineData("Release", false)] | ||
| public async Task AllocateLargeHeapThenRepeatedlyInterop(string config, bool buildNative) | ||
| { | ||
| // 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" | ||
| CopyTestAsset("WasmBasicTestApp", "MemoryTests", "App"); | ||
| string extraArgs = $"-p:EmccMaximumHeapSize=4294901760 -p:WasmBuildNative={buildNative}"; | ||
| BuildProject(config, assertAppBundle: false, extraArgs: extraArgs); | ||
|
|
||
| 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")); | ||
| } | ||
| } |
58 changes: 58 additions & 0 deletions
58
src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.IO; | ||
| using System.Text.Json; | ||
| using System.Runtime.InteropServices.JavaScript; | ||
|
|
||
| public partial class MemoryTest | ||
| { | ||
| [JSImport("joinStringArray", "main.js")] | ||
| internal static partial string JoinStringArray(string[] testArray); | ||
|
|
||
| [JSExport] | ||
| internal static void Run() | ||
| { | ||
| // Allocate a 2GB space (20 int arrays of 100MB, 100MB = 4 * 1024 * 1024 * 25) | ||
| const int arrayCnt = 20; | ||
| int[][] arrayHolder = new int[arrayCnt][]; | ||
| string errors = ""; | ||
| for (int i = 0; i < arrayCnt; i++) | ||
| { | ||
| try | ||
| { | ||
| arrayHolder[i] = new int[1024 * 1024 * 25]; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| errors += $"Exception {ex} was thrown on i={i}"; | ||
| } | ||
| } | ||
|
|
||
| // marshall string array to JS | ||
| string [] testArray = new [] { "M", "e", "m", "o", "r", "y", "T", "e", "s", "t" }; | ||
| string response = JoinStringArray(testArray); | ||
|
|
||
| bool correct = AssertJoinCorrect(testArray, response); | ||
| if (!correct) | ||
| errors += $"expected response: {response}, testArray: {string.Join("", testArray)}"; | ||
|
|
||
| // call a method many times to trigger tier-up optimization | ||
| for (int i = 0; i < 10000; i++) | ||
| { | ||
| AssertJoinCorrect(testArray, response); | ||
| } | ||
| if (!string.IsNullOrEmpty(errors)) | ||
| { | ||
| TestOutput.WriteLine(errors); | ||
| throw new Exception(errors); | ||
| } | ||
| } | ||
|
|
||
| private static bool AssertJoinCorrect(string[] testArray, string expected) | ||
| { | ||
| string joinedArray = string.Join("", testArray); | ||
| return joinedArray.Equals(expected); | ||
| } | ||
| } | ||
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make each string here random (constants are not allocated) and large enough, so that the JS side is forced to make allocations above 2GB mark. And overflow to negative numbers. Maybe run this in a loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's use the
arrayHolderthat is already prepared.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2GB string allocation with random data is taking too long, not only to run WBT but to even debug it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we cannot just take
arrayHolderand send it to JS to force JS to allocate 2GB because interop does not support array of arrays. Allocating flat array of 2GB requires more linear memory and can be problematic from other reasons than investigated in the issue. Allocating an array of strings theoretically should work but as I already mentioned, it takes tens of minutes to finish the allocation, even usingStringBuilder.@maraf, do you have a good idea how to fulfill the requirements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I realized that the key point was "allocations above 2GB mark", not "allocations of 2GB".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're allocating
2 621 440 000 bytesso more than max int. There are no traces of overflow. Let's merge this to take care of the relink fix PR that is built on top of this.