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.

Sign up for GitHub

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

Make wasm-interpreter work like wasm-aot #1769

Merged
merged 5 commits into from
Aug 12, 2021
Merged

Make wasm-interpreter work like wasm-aot #1769

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff61a7d
Made interpreter work more like wasm-aot.
Aug 10, 2021
5eabb34
Removed wasmmainJS option.
Aug 10, 2021
9f5523f
Removed temp files.
Aug 10, 2021
214e1dd
Removed trim analysis supression.
Aug 10, 2021
436cbb1
Removed no longer used WasmMainJS parameter.
Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,6 @@ public class CommandLineOptions
[Option("wasmEngine", Required = false, HelpText = "Full path to a java script engine used to run the benchmarks, used by Wasm toolchain.")]
public FileInfo WasmJavascriptEngine { get; set; }

[Option("wasmMainJS", Required = false, HelpText = "Path to the main.js file used by Wasm toolchain. Mandatory when using \"--runtimes wasm\"")]
public FileInfo WasmMainJs { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep in mind that once we delete this parameter and you update BDN in dotnet/performance everything that is still using this argument is going to get an error from the BDN config parser

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am aware. We have made changes in dotnet/performance so nothing is using it.


[Option("wasmArgs", Required = false, Default = "--expose_wasm", HelpText = "Arguments for the javascript engine used by Wasm toolchain.")]
public string WasmJavaScriptEngineArguments { get; set; }

Expand Down
12 changes: 2 additions & 10 deletions src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,11 @@ private static bool Validate(CommandLineOptions options, ILogger logger)
logger.WriteLineError($"The provided runtime \"{runtime}\" is invalid. Available options are: {string.Join(", ", Enum.GetNames(typeof(RuntimeMoniker)).Select(name => name.ToLower()))}.");
return false;
}
else if (runtimeMoniker == RuntimeMoniker.Wasm && options.WasmMainJs == null && options.RuntimeSrcDir == null) {
logger.WriteLine("Either runtimeSrcDir or WasmMainJS must be specified.");
}
else if (runtimeMoniker == RuntimeMoniker.Wasm && options.WasmMainJs.IsNotNullButDoesNotExist())
{
logger.WriteLineError($"The provided {nameof(options.WasmMainJs)} \"{options.WasmMainJs}\" does NOT exist.");
return false;
}
else if (runtimeMoniker == RuntimeMoniker.MonoAOTLLVM && (options.AOTCompilerPath == null || options.AOTCompilerPath.IsNotNullButDoesNotExist()))
{
logger.WriteLineError($"The provided {nameof(options.AOTCompilerPath)} \"{ options.AOTCompilerPath }\" does NOT exist. It MUST be provided.");
}
else if (runtimeMoniker == RuntimeMoniker.Wasm && options.AOTCompilerMode == MonoAotCompilerMode.wasm && (options.RuntimeSrcDir == null || options.RuntimeSrcDir.IsNotNullButDoesNotExist()))
else if (runtimeMoniker == RuntimeMoniker.Wasm && (options.RuntimeSrcDir == null || options.RuntimeSrcDir.IsNotNullButDoesNotExist()))
{
logger.WriteLineError($"The provided {nameof(options.RuntimeSrcDir)} \"{options.RuntimeSrcDir}\" does NOT exist. It MUST be provided for wasm-aot.");
return false;
Expand Down Expand Up @@ -425,7 +417,7 @@ private static Job MakeWasmJob(Job baseJob, CommandLineOptions options, TimeSpan
bool wasmAot = options.AOTCompilerMode == MonoAotCompilerMode.wasm;

var wasmRuntime = new WasmRuntime(
mainJs: options.WasmMainJs ?? new FileInfo(Path.Combine(options.RuntimeSrcDir.FullName, "src", "mono", "wasm", "runtime-test.js")),
mainJs: new FileInfo(Path.Combine(options.RuntimeSrcDir.FullName, "src", "mono", "wasm", "runtime-test.js")),
naricc marked this conversation as resolved.
Show resolved Hide resolved
msBuildMoniker: msBuildMoniker,
javaScriptEngine: options.WasmJavascriptEngine?.FullName ?? "v8",
javaScriptEngineArguments: options.WasmJavaScriptEngineArguments,
Expand Down
25 changes: 10 additions & 15 deletions src/BenchmarkDotNet/Templates/WasmCsProj.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,32 @@
<Project Sdk="$SDKNAME$" DefaultTargets="$TARGET$">
<Project Sdk="$SDKNAME$" DefaultTargets="publish">
<PropertyGroup>
<OutputType>Exe</OutputType>
<OutputPath>bin</OutputPath>
<RuntimeSrcDir>$RUNTIMESRCDIR$</RuntimeSrcDir>
<RuntimeConfig>Release</RuntimeConfig>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<TargetFramework>$TFM$</TargetFramework>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<AppDir>$(MSBuildThisFileDirectory)\bin\$TFM$\browser-wasm\publish</AppDir>
<AssemblyName>$PROGRAMNAME$</AssemblyName>
<RuntimeIdentifier>browser-wasm</RuntimeIdentifier>
<WasmMainJSPath>$MAINJS$</WasmMainJSPath>
<MicrosoftNetCoreAppRuntimePackDir>$RUNTIMEPACK$</MicrosoftNetCoreAppRuntimePackDir>
<UsingBrowserRuntimeWorkload>false</UsingBrowserRuntimeWorkload>
<PublishTrimmed>false</PublishTrimmed>
<RunAOTCompilation>false</RunAOTCompilation>
<WasmMainJSPath>$(RuntimeSrcDir)\src\mono\wasm\runtime-test.js</WasmMainJSPath>
<WasmGenerateRunV8Script>true</WasmGenerateRunV8Script>
<ValidateExecutableReferencesMatchSelfContained>false</ValidateExecutableReferencesMatchSelfContained>
<WasmNativeWorkload>false</WasmNativeWorkload>
<UsingBrowserRuntimeWorkload>false</UsingBrowserRuntimeWorkload>
$COPIEDSETTINGS$
</PropertyGroup>

<Import Project="$(RuntimeSrcDir)/src/mono/wasm/build/WasmApp.LocalBuild.props" />
<Import Project="$(RuntimeSrcDir)/src/mono/wasm/build/WasmApp.LocalBuild.targets" />

<ItemGroup>
<Compile Include="$CODEFILENAME$" Exclude="bin\**;obj\**;**\*.xproj;packages\**" />
</ItemGroup>

<Target Name="UpdateRuntimePack"
DependsOnTargets="ResolveFrameworkReferences" Condition="'$(MicrosoftNetCoreAppRuntimePackDir)' != ''">
<ItemGroup>
<ResolvedRuntimePack Update="@(ResolvedRuntimePack)" PackageDirectory="$(MicrosoftNetCoreAppRuntimePackDir)" />
</ItemGroup>
</Target>

<Target Name="PublishWithCustomRuntimePack"
AfterTargets="Build"
DependsOnTargets="UpdateRuntimePack;Publish" />

<ItemGroup>
<ProjectReference Include="$CSPROJPATH$" />
</ItemGroup>
Expand Down
53 changes: 0 additions & 53 deletions src/BenchmarkDotNet/Toolchains/MonoWasm/WasmBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,61 +28,8 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart

WasmRuntime runtime = (WasmRuntime)buildPartition.Runtime;

if (buildResult.IsBuildSuccess && !runtime.Aot)
{
BuildApp(buildPartition.ProgramName, generateResult.ArtifactsPaths.BuildArtifactsDirectoryPath, runtime);
}

return buildResult;
}

private void BuildApp(string programName, string projectRoot, WasmRuntime runtime)
{
string appDir = Path.Combine(projectRoot, $"bin", targetFrameworkMoniker, "browser-wasm", "publish");
string outputDir = Path.Combine(appDir, "output");

string mainAssemblyPath = Path.Combine(appDir, $"{programName}.dll");

if (!File.Exists(mainAssemblyPath))
throw new ArgumentException($"File MainAssembly='{mainAssemblyPath}' doesn't exist.");

var assemblies = Directory.GetFiles(appDir, "*.dll");

// Create app
Directory.CreateDirectory(outputDir);
Directory.CreateDirectory(Path.Combine(outputDir, "managed"));
foreach (var assembly in assemblies)
File.Copy(assembly, Path.Combine(outputDir, "managed", Path.GetFileName(assembly)), true);

string timeZoneDat = "dotnet.timezones.blat";
string icutDat = "icudt.dat";

foreach (var f in new string[] { "dotnet.wasm", "dotnet.js", timeZoneDat, icutDat })
File.Copy(Path.Combine(appDir, f), Path.Combine(outputDir, f), true);

File.Copy(runtime.MainJs.FullName, Path.Combine(outputDir, "runtime.js"), true);

using (var sw = File.CreateText(Path.Combine(outputDir, "mono-config.js")))
{
sw.WriteLine("config = {");
sw.WriteLine("\t\"assembly_root\": \"managed\",");
sw.WriteLine("\t\"enable_debugging\": 0,");
sw.WriteLine("\t\"assets\": [");

foreach (var assembly in assemblies)
{
sw.Write($"\t\t{{ \"behavior\": \"assembly\", \"name\": \"{ Path.GetFileName(assembly)}\" }}");
sw.WriteLine(",");
}

sw.WriteLine($"\t\t{{ \"behavior\": \"icu\", \"name\": \"icudt.dat\", \"load_remote\": false}},");
sw.WriteLine($"\t\t{{ \"behavior\": \"vfs\", \"name\": \"{timeZoneDat}\", \"virtual_path\": \"/usr/share/zoneinfo/\" }}");

sw.WriteLine("\t],");
sw.WriteLine("\t\"files_to_map\": []");

sw.WriteLine("};");
}
}
}
}
7 changes: 0 additions & 7 deletions src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,7 @@ protected void GenerateProjectInterpreter(BuildPartition buildPartition, Artifac

protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
{
if (Aot)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change and I would like to be sure that we are not breaking anything. Have you tested both AOT and non-AOT toolchains?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, before wasm-intepreter mode was using a simplified custom app builder I built into BDN, because that was the best thing that could be done at the time.

Wasm-aot used all the app bundling stuff from the the runtime source directory, which put things in a different place than the one I coded into BDN.

With this change, both wasm-interpreter and wasm-aot will use the app builder from the runtime source dir, so the files will end up in the same place.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested both AOT and interpreter configurations.

{
return Path.Combine(buildArtifactsDirectoryPath, "bin", TargetFrameworkMoniker, "browser-wasm", "AppBundle");
}
else
{
return Path.Combine(buildArtifactsDirectoryPath, "bin", TargetFrameworkMoniker, "browser-wasm", "publish", "output");
}
}
}
}