[browser] prevent <DebuggerSupport> from disabling interp optimizations (#85792)
authorPavel Savara <pavel.savara@gmail.com>
Fri, 5 May 2023 17:03:24 +0000 (19:03 +0200)
committerGitHub <noreply@github.com>
Fri, 5 May 2023 17:03:24 +0000 (13:03 -0400)
* [wasm] Don't enable debugging for library tests by default

@pavelsavara and @brzvlad found that some tests like
System.Collections.Immutable.Tests run slower because the runtime has
debugging enabled which causes the interpreter to disable optimizations.

Reason:

- Some tests depend on debugger attributes, and thus set
  `$(DebuggerSupport)=true` to preserve them while linking.
- Which sets `$(WasmDebugLevel)=-1`. But setting `$(WasmDebugLevel)<0`
  enables debugging support in the runtime.

- So, set this to `<0` only when building with `Configuration=Debug` .

* set it to zero only on CI

---------

Co-authored-by: Ankit Jain <radical@gmail.com>
eng/testing/tests.browser.targets
eng/testing/tests.wasi.targets
eng/testing/tests.wasm.targets

index ac4796a..d358982 100644 (file)
       <WasmGenerateRunV8Script>true</WasmGenerateRunV8Script>
 
       <WasmNativeDebugSymbols Condition="'$(DebuggerSupport)' == 'true' and '$(WasmNativeDebugSymbols)' == ''">true</WasmNativeDebugSymbols>
-      <WasmDebugLevel Condition="'$(DebuggerSupport)' == 'true' and '$(WasmDebugLevel)' == ''">-1</WasmDebugLevel>
+      <!--
+        Do this *after* importing WasmApp.targets. tests.wasm.targets sets this to `reset-to-zero` to indicate
+        that we want to force this value to zero.
+
+        WasmApp.targets *overrides* `WasmDebugLevel` when `DebuggerSupport=true`, but for the library tests
+        we explicitly want to:
+        1. build with DebuggerSupport=true so the debugger attributes are preserved by the linker;
+        2. *debugging* is disabled at run time so the interpreter optimizations don't get disabled.
+      -->
+      <WasmDebugLevel Condition="'$(WasmDebugLevel)' == 'reset-to-zero'">0</WasmDebugLevel>
     </PropertyGroup>
 
     <ItemGroup Condition="'$(IncludeSatelliteAssembliesInVFS)' == 'true' and '$(BuildAOTTestsOnHelix)' != 'true'">
index 87dbe5f..14b217b 100644 (file)
       <WasmInvariantGlobalization>$(InvariantGlobalization)</WasmInvariantGlobalization>
 
       <WasmNativeDebugSymbols Condition="'$(DebuggerSupport)' == 'true' and '$(WasmNativeDebugSymbols)' == ''">true</WasmNativeDebugSymbols>
-      <WasmDebugLevel Condition="'$(DebuggerSupport)' == 'true' and '$(WasmDebugLevel)' == ''">-1</WasmDebugLevel>
+      <!--
+        Do this *after* importing WasmApp.targets. tests.wasm.targets sets this to `reset-to-zero` to indicate
+        that we want to force this value to zero.
+
+        WasmApp.targets *overrides* `WasmDebugLevel` when `DebuggerSupport=true`, but for the library tests
+        we explicitly want to:
+        1. build with DebuggerSupport=true so the debugger attributes are preserved by the linker;
+        2. *debugging* is disabled at run time so the interpreter optimizations don't get disabled.
+      -->
+      <WasmDebugLevel Condition="'$(WasmDebugLevel)' == 'reset-to-zero'">0</WasmDebugLevel>
     </PropertyGroup>
 
     <ItemGroup Condition="'$(IncludeSatelliteAssembliesInVFS)' == 'true' and '$(BuildAOTTestsOnHelix)' != 'true'">
index 9c01950..4d6abbf 100644 (file)
@@ -6,6 +6,20 @@
     <ArchiveTests Condition="'$(WasmBuildingForNestedPublish)' == 'true'">false</ArchiveTests>
     <BundleTestAppTargets>$(BundleTestAppTargets);BundleTestWasmApp</BundleTestAppTargets>
     <DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(Configuration)' == 'Debug'">true</DebuggerSupport>
+    <!--
+        Some tests depend on debugger attributes, and thus set $(DebuggerSupport)=true to preserve
+        them while linking.
+        But setting WasmDebugLevel<0 disables optimizations in the interpreter. So setting that
+        based on $(DebuggerSupport) has unintended slow down.
+
+        But we do want to set it for Configuration=Debug .
+    -->
+    <WasmDebugLevel Condition="'$(Configuration)' == 'Debug' and '$(WasmDebugLevel)' == ''">-1</WasmDebugLevel>
+    <!-- We want to have WasmDebugLevel=0, but if DebuggerSupport=true then WasmApp.targets
+         *will* override it. So, set this to "reset-to-zero" here, and reset the value later
+         in targets. -->
+    <WasmDebugLevel Condition="'$(DebuggerSupport)' == 'true' and '$(ContinuousIntegrationBuild)' == 'true' and '$(WasmDebugLevel)' == ''">reset-to-zero</WasmDebugLevel>
+
     <TrimMode Condition="'$(TrimMode)' == ''">full</TrimMode>
 
     <!-- Some tests expect to load satellite assemblies by path, eg. System.Runtime.Loader.Tests,