Open
Conversation
Collaborator
Looking at the code, |
Contributor
Author
It is used right here which then sets the I have a ci run without the last commit on main, that worked. It seems like the switch to the |
timcassell
reviewed
Feb 23, 2026
Collaborator
Oh, duh, not sure how I missed that. What a mess, that could use some cleanup, too. So the test was never actually testing aot. |
Collaborator
Maybe the wwwroot is necessary? I also have a green CI run where I had it running on node, with minimal other changes to the toolchain (besides the async refactor and mjs changes for websocket). |
FreyLuis
force-pushed
the
improve-wasm
branch
from
d588ae6 to
164b818
Compare
Contributor
Author
|
I don't know why the build doesn't fail in the ci, it crashes on my windows machine with the following errors: It does work with a minimal project (including main.js path). I'll see if i can fix this tomorrow. |
… from PATH - resolve js runtime from path - add option to specify wasm main.js template - add custom js runtime argument formatter option
FreyLuis
force-pushed
the
improve-wasm
branch
from
2ead977 to
db48498
Compare
Contributor
Author
|
@timcassell It works now. The IntigrationTests still fail while building the dllgatherer on windows on my pc, but i couldn't reproduce it on another machine or with another project. Like I wrote in the description, this does remove the I've noticed an odd behavior that I did not want to touch: When I set KeepBenchmarkFiles to true and build the project twice, it references all dlls from the previous build. I'm not sure if BenchmarkDotNet should delete the dlls before reusing the folder or build in another folder instead. |
Collaborator
What's the error?
|
Contributor
Author
Looks like the error happened while building BenchmarkDotNet.Autogenerated. The dllgatherer only crashed because I didn't build the project before running the IntigrationTests. All those files do exist. BenchmarkDotNet.IntegrationTests-1-build-no-restore-no-deps.binlog.zip |
Collaborator
|
Looks like there's a merge conflict. |
FreyLuis
force-pushed
the
improve-wasm
branch
from
f2c678d to
58a89b1
Compare
Contributor
Author
Should be fixed now. https://github.com/twobrainsgmbh/BenchmarkDotNet/actions/runs/22404078490 |
timcassell
reviewed
Feb 26, 2026
timcassell
reviewed
Feb 26, 2026
| <WasmTargetsPath>$([System.IO.Path]::ChangeExtension('$(OriginalCSProjPath)', '.Wasm.targets'))</WasmTargetsPath> | ||
| <WasmDataDir>$WASMDATADIR$</WasmDataDir> | ||
| <WasmDataDir Condition="'$(WasmDataDir)' != ''">$([MSBuild]::NormalizeDirectory($(WasmDataDir)))</WasmDataDir> | ||
| <WasmMainJSPath>$MAINJS$</WasmMainJSPath> |
Collaborator
There was a problem hiding this comment.
@ilonatommy You recently changed away from this property, what are your thoughts here?
timcassell
reviewed
Feb 26, 2026
timcassell
reviewed
Feb 26, 2026
timcassell
reviewed
Feb 26, 2026
timcassell
reviewed
Feb 26, 2026
timcassell
reviewed
Feb 26, 2026
tests/BenchmarkDotNet.IntegrationTests/BenchmarkDotNet.IntegrationTests.csproj
Show resolved
Hide resolved
Collaborator
pavelsavara
approved these changes
Feb 27, 2026
Member
pavelsavara
left a comment
pavelsavara
left a comment
There was a problem hiding this comment.
LGTM, but wait for Ilona's review
| if (globalThis.arguments !== undefined) | ||
| return globalThis.arguments; | ||
|
|
||
| // spdermonkey |
| [Theory] | ||
| [InlineData(MonoAotCompilerMode.mini)] | ||
| [InlineData(MonoAotCompilerMode.wasm)] | ||
| [InlineData(MonoAotCompilerMode.wasm, Skip = "AOT is broken")] |
Member
There was a problem hiding this comment.
Add link to active issue. What's wrong with it ?
timcassell
reviewed
Feb 27, 2026
| File.WriteAllText(targetMainJsPath, content); | ||
| } | ||
|
|
||
| protected override string GetExecutablePath(string binariesDirectoryPath, string programName) => Path.Combine(binariesDirectoryPath, "wwwroot", "main.js"); |
Collaborator
There was a problem hiding this comment.
nit: I think the modern convention is to use .mjs extension.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR aims to improve WASM support (as described in #2890) in a few ways:
JavaScriptEnginenow resolves executables from the PATH.mainJsTemplatePathoption allows customizing the startup script (with polyfill, for example).--wasmDataDiroption, that was used by dotnet/performance to setWasmMainJSPath.--expose_wasmas default args forjavaScriptEngineArguments.AOT currently fails because the unit tests set only MonoAotCompilerMode and not the WasmRuntimes aot option. I've fixed the test and disabled it for now.