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

[mono][wasm] Avoid AOTing methods with clauses, use the interpreter instead. #52883

Merged
merged 6 commits into from
Jun 7, 2021

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented May 18, 2021

No description provided.

@ghost
Copy link

ghost commented May 18, 2021

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details
Author: vargaz
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@vargaz vargaz added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label May 18, 2021
@vargaz vargaz marked this pull request as draft May 18, 2021 02:48
@radekdoulik
Copy link
Member

Tried to run the browser-bench sample on chrome/amd64 to compare it to #52846. It crashes in the TryCatchThrow without any information in the console.

So I used firefox instead and remeasured the #52846 and this PR.

firefox/amd64/AOT:

measurement before after
Exceptions, NoExceptionHandling 0.0148us 0.0151us
Exceptions, TryCatch 0.1541us 0.3796us
Exceptions, TryCatchThrow 0.0134ms 0.0232ms
Exceptions, TryCatchFilter 0.3844us 0.4336us
Exceptions, TryCatchFilterInline 0.1423us 0.1422us
Json, non-ASCII text serialize 1.1937ms 1.4946ms
Json, non-ASCII text deserialize 5.3005ms 5.4476ms
Json, small serialize 0.1945ms 0.2138ms
Json, small deserialize 0.2243ms 0.2996ms
Json, large serialize 51.7475ms 56.7634ms
Json, large deserialize 59.5862ms 79.8235ms

It also looks like the JS rountrip overhead is lower on firefox compared to chrome.

@radekdoulik
Copy link
Member

I will also try to use it with the test case from #52394

@vargaz vargaz force-pushed the wasm-no-eh branch 2 times, most recently from ad8b3cb to fd9cd71 Compare May 18, 2021 23:28
@akoeplinger
Copy link
Member

Could we emit a log message in the AOT compiler when we hit such methods to make it easier to extract them?

@radekdoulik
Copy link
Member

Nice, the recent changes fix the crashes in chrome and also in the disabled measurements.

The times look like this now:

measurement before after
Exceptions, NoExceptionHandling 0.0166us 0.0165us
Exceptions, TryCatch 0.1413us 0.4183us
Exceptions, TryCatchThrow 0.0065ms 0.0126ms
Exceptions, TryCatchFilter 0.4460us 0.4263us
Exceptions, TryCatchFilterThrow 0.0138ms
Exceptions, TryCatchFilterThrowApplies 0.0127ms
Exceptions, TryCatchFilterInline 0.1287us 0.1244us
Json, non-ASCII text serialize 1.4873ms 1.4281ms
Json, non-ASCII text deserialize 6.7141ms 6.5593ms
Json, small serialize 0.2106ms 0.2017ms
Json, small deserialize 0.2376ms 0.2790ms
Json, large serialize 55.5699ms 52.8144ms
Json, large deserialize 64.3457ms 75.0725ms

@lewing
Copy link
Member

lewing commented May 19, 2021

If looks like maybe the ascii json deserialize path does a lot of work in a method with a clause?

@lewing
Copy link
Member

lewing commented May 19, 2021

@vargaz is the gc fix more general than this pr?

@vargaz
Copy link
Contributor Author

vargaz commented May 19, 2021

Yes.

@lewing
Copy link
Member

lewing commented May 20, 2021

Most of the failures are resolved but there are still a few kinds

[00:42:33] info: console.debug: MONO_WASM: ICU data archive(s) loaded, disabling invariant mode
[00:42:33] info: console.debug: mono_wasm_runtime_ready fe00e07a-5519-4dfe-b35a-f867dbaf2e28
[00:42:33] info: console.info: Initializing.....
[00:42:33] info: Discovering: System.Net.Http.Unit.Tests.dll (method display = ClassAndMethod, method display options = None)
[00:42:33] info: Discovered:  System.Net.Http.Unit.Tests.dll (found 902 of 906 test cases)
[00:42:33] info: Starting:    System.Net.Http.Unit.Tests.dll
[00:42:34] fail: [FAIL] System.Net.Http.Tests.HttpHeadersTest.GetEnumerator_UseExplicitInterfaceImplementation_EnumeratorReturnsNoOfHeaders
[00:42:34] info: Assert.True() Failure
[00:42:34] info: Expected: True
[00:42:34] info: Actual:   False
[00:42:34] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:42:34] fail: [FAIL] System.Net.Http.Tests.HttpHeadersTest.GetEnumerator_FirstHeaderWithOneValueSecondHeaderWithTwoValues_EnumeratorReturnsTwoHeaders
[00:42:34] info: Assert.True() Failure
[00:42:34] info: Expected: True
[00:42:34] info: Actual:   False
[00:42:34] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:42:34] fail: [FAIL] System.Net.Http.Tests.HttpHeadersTest.GetEnumerator_FirstCustomHeaderWithEmptyValueSecondKnownHeaderWithEmptyValue_EnumeratorReturnsOneHeader
[00:42:34] info: Assert.True() Failure
[00:42:34] info: Expected: True
[00:42:34] info: Actual:   False
[00:42:34] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:42:48] info: Finished:    System.Net.Http.Unit.Tests.dll
[00:42:48] info: 
[00:42:48] info: === TEST EXECUTION SUMMARY ===
[00:42:48] info: Total: 2021, Errors: 0, Failed: 3, Skipped: 17, Time: 14.933803s
[00:42:48] info: 
[00:42:48] info: Process v8 exited with 1


[00:50:23] info: Discovering: Microsoft.VisualBasic.Core.Tests.dll (method display = ClassAndMethod, method display options = None)
[00:50:23] info: Discovered:  Microsoft.VisualBasic.Core.Tests.dll (found 704 of 719 test cases)
[00:50:23] info: Starting:    Microsoft.VisualBasic.Core.Tests.dll
[00:50:27] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.Write_ArgumentException
[00:50:27] info: System.IO.IOException : Bad file name or number.
[00:50:27] info:    at Microsoft.VisualBasic.FileSystem.Write(Int32 , Object[] )
[00:50:27] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.<>c__DisplayClass21_0.<Write_ArgumentException>b__0()
[00:50:27] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.AssertThrows[ArgumentException](Action action)
[00:50:27] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:50:27] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.FileClose
[00:50:27] info: Assert.Equal() Failure
[00:50:27] info: Expected: 2
[00:50:27] info: Actual:   1
[00:50:27] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:50:27] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.FileOpen
[00:50:27] info: Assert.Throws() Failure
[00:50:27] info: Expected: typeof(System.IO.IOException)
[00:50:27] info: Actual:   (No exception was thrown)
[00:50:27] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.AssertThrows[IOException](Action action)
[00:50:27] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:50:45] info: Finished:    Microsoft.VisualBasic.Core.Tests.dll
[00:50:45] info: 


                 
                 
[00:51:17] info: console.info: Arguments: --run,WasmTestRunner.dll,System.Text.RegularExpressions.Tests.dll,-notrait,category=IgnoreForCI,-notrait,category=OuterLoop,-notrait,category=failing
[00:51:17] info: console.debug: MONO_WASM: Initializing mono runtime
[00:51:17] info: console.debug: MONO_WASM: ICU data archive(s) loaded, disabling invariant mode
[00:51:17] info: console.debug: mono_wasm_runtime_ready fe00e07a-5519-4dfe-b35a-f867dbaf2e28
[00:51:17] info: console.info: Initializing.....
[00:51:17] info: Discovering: System.Text.RegularExpressions.Tests.dll (method display = ClassAndMethod, method display options = None)
[00:51:17] info: Discovered:  System.Text.RegularExpressions.Tests.dll (found 200 of 218 test cases)
[00:51:17] info: Starting:    System.Text.RegularExpressions.Tests.dll
[00:51:17] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexSplitTests.Split
[00:51:17] info: System.Exception : Test method 'Split_TestData' not found
[00:51:17] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:17] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:23] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexMultipleMatchTests.Matches
[00:51:23] info: System.Exception : Test method 'Matches_TestData' not found
[00:51:23] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:23] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:35] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexMatchTests.Match_Advanced
[00:51:35] info: System.Exception : Test method 'Match_Advanced_TestData' not found
[00:51:35] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:35] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:35] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexMatchTests.Match
[00:51:35] info: System.Exception : Test method 'Match_Basic_TestData' not found
[00:51:35] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:35] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:35] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexMatchTests.Match_StartatDiffersFromBeginning
[00:51:35] info: System.Exception : Test method 'Match_StartatDiffersFromBeginning_MemberData' not found
[00:51:35] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:35] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:38] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexReplaceTests.Replace
[00:51:38] info: System.Exception : Test method 'Replace_String_TestData' not found
[00:51:38] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:38] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:38] fail: [FAIL] System.Text.RegularExpressions.Tests.RegexReplaceTests.Replace_MatchEvaluator_Test
[00:51:38] info: System.Exception : Test method 'Replace_MatchEvaluator_TestData' not found
[00:51:38] info:    at System.Text.RegularExpressions.Tests.RegexCompilationHelper.TransformRegexOptions(String testDataMethodName, Int32 regexOptionsArrayIndex)
[00:51:38] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[00:51:38] info: Finished:    System.Text.RegularExpressions.Tests.dll
[00:51:38] info: 
[00:51:38] info: === TEST EXECUTION SUMMARY ===
[00:51:38] info: Total: 6541, Errors: 0, Failed: 7, Skipped: 12, Time: 21.1881339s
[00:51:38] info: 
[00:51:39] info: Process v8 exited with 1

@lewing
Copy link
Member

lewing commented May 25, 2021

@vargaz any idea what is going on with the remaining failures?

@vargaz
Copy link
Contributor Author

vargaz commented May 26, 2021

The Microsoft.VisualBasic failures seem to be related to calls made to Assembly.GetCallingAssembly () which is not supported in wasm+aot, not sure why this worked previously.

@vargaz
Copy link
Contributor Author

vargaz commented May 26, 2021

Testcase for the System.Net.Http failures:

public class Test : IEnumerable<string>
{
        public IEnumerator<string> GetEnumerator()
        {
            var l = new List<string> ();
            l.Add ("A");
            foreach (var v in l)
                yield return "FOO";
        }

        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public static void Main () {
        var t = new Test ();
        var en = t.GetEnumerator ();
        Console.WriteLine ("V: " + en.MoveNext ());
    }
}

@radical
Copy link
Member

radical commented May 26, 2021

@vargaz is the gc fix more general than this pr?

Is that just this commit - ba54702 ? Should I expect it to work, if cherry-picked to the AOT test PR?

@vargaz
Copy link
Contributor Author

vargaz commented May 26, 2021

@vargaz is the gc fix more general than this pr?

Is that just this commit - ba54702 ? Should I expect it to work, if cherry-picked to the AOT test PR?

Yes

@vargaz
Copy link
Contributor Author

vargaz commented May 27, 2021

The only failures left are the ones in Microsoft.VisualBasic.Core.Tests. I think those tests should be disabled because of their usage of Assembly.GetCallingAssembly ().

@lewing
Copy link
Member

lewing commented May 27, 2021

@radekdoulik can we get updated timings now?

@lewing
Copy link
Member

lewing commented May 27, 2021

@vargaz are you in favor of taking the change?

@vargaz vargaz reopened this Jun 2, 2021
@vargaz vargaz marked this pull request as ready for review June 2, 2021 13:41
@vargaz vargaz added arch-wasm WebAssembly architecture and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) labels Jun 3, 2021
@ghost
Copy link

ghost commented Jun 3, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
Author: vargaz
Assignees: -
Labels:

arch-wasm, area-Codegen-Interpreter-mono

Milestone: -

@vargaz
Copy link
Contributor Author

vargaz commented Jun 3, 2021

Still some relevant failures.

…nstead.

This will hopefully fix semantical problems with filter clauses.
… cannot be handled in the frames below the interp_entry () call.
Stack walks are not supported on wasm, and the workarounds depend on inserting code into the
caller, i.e. callers of GetExecutingAssembly (), so if the caller is interpreted, the workaround
is not generated.
@radekdoulik
Copy link
Member

Nice to see it in green, great work @vargaz.

Fresh measurements on Chrome/AOT against main with the same hash this was rebased at:

measurement main PR
Exceptions, NoExceptionHandling 0.0163us 0.0164us
Exceptions, TryCatch 0.0943us 0.2914us
Exceptions, TryCatchThrow 0.0047ms 0.0091ms
Exceptions, TryCatchFilter 0.3429us 0.3074us
Exceptions, TryCatchFilterInline 0.1095us 0.1099us
Json, non-ASCII text serialize 1.3103ms 1.3386ms
Json, non-ASCII text deserialize 5.4513ms 5.3631ms
Json, small serialize 0.1657ms 0.1649ms
Json, small deserialize 0.1911ms 0.2291ms
Json, large serialize 44.8772ms 44.7281ms
Json, large deserialize 51.5842ms 61.5765ms

@radical
Copy link
Member

radical commented Jun 7, 2021

AOT runtime-staging failures:

  1. System.Runtime.Serialization.Xml.ReflectionOnly.Tests - timing out, same as main, likely caused by the 2.0.21 Emscripten bump
  2. System.Runtime.Serialization.Xml.Tests - same as above
  3. Microsoft.VisualBasic.Core.Tests(log) - this is not failing on main:
[18:33:08] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.Write_ArgumentException
[18:33:08] info: System.IO.IOException : Bad file name or number.
[18:33:08] info:    at Microsoft.VisualBasic.FileSystem.Write(Int32 , Object[] )
[18:33:08] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.<>c__DisplayClass21_0.<Write_ArgumentException>b__0()
[18:33:08] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.AssertThrows[ArgumentException](Action action)
[18:33:08] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[18:33:08] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.FileClose
[18:33:08] info: Assert.Equal() Failure
[18:33:08] info: Expected: 2
[18:33:08] info: Actual:   1
[18:33:08] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[18:33:08] fail: [FAIL] Microsoft.VisualBasic.Tests.FileSystemTests.FileOpen
[18:33:08] info: Assert.Throws() Failure
[18:33:08] info: Expected: typeof(System.IO.IOException)
[18:33:08] info: Actual:   (No exception was thrown)
[18:33:08] info:    at Microsoft.VisualBasic.Tests.FileSystemTests.AssertThrows[IOException](Action action)
[18:33:08] info:    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )
[18:33:27] info: Finished:    Microsoft.VisualBasic.Core.Tests.dll
[18:33:27] info: 
[18:33:27] info: === TEST EXECUTION SUMMARY ===
[18:33:27] info: Total: 22222, Errors: 0, Failed: 3, Skipped: 3, Time: 25.016673s

@vargaz
Copy link
Contributor Author

vargaz commented Jun 7, 2021

for the visualbasic failure, see:
#52883 (comment)

@radical radical merged commit d173cca into dotnet:main Jun 7, 2021
@vargaz vargaz deleted the wasm-no-eh branch June 8, 2021 00:07
@ghost ghost locked as resolved and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants