WASM Microbenchmarks Not Working #2250
Labels
bug
Something isn't working
impact test coverage
test maybe disabled due to this issue
WRI
Issues and PR's that may be especially useful to WRI or should be viewed during WRI
Comments
LoopedBard3
added
bug
|
@pavelsavara @lewing - please take a look |
|
Yes |
|
This should be resovled with this change to BDN: dotnet/BenchmarkDotNet#1916 Are it looks like the update to pull the new version of BDN merged: #2245 So why is this still broken with the same error message? Did we get the wrong BDN version? |
|
@pavelsavara This run specifies to target net7.0. The inner build command is: giving this output |
|
I retrieved the command it is running the microbenchmakrs with from the logs: Trying this command locally (with modifications. for the paths), and it seems to run fine. So I am not sure where the error is coming in. Are these runs happening with the very latest Microbenchmarks.csproj? I will retrieve the helix payload and see what I can get out of it. |
LoopedBard3
added
the
WRI
|
dotnet/BenchmarkDotNet#1926 should help |
|
The bdn change has merged, what are the next steps? |
|
@LoopedBard3 @DrewScoggins Can one of you update the BDN version the microbenchmarks pull, and upload the new package to the repository? |
|
PR to update the version is up: #2270. |
|
It looks like even after the update, the main.js file is the one being looked for. |
Looking at dotnet/BenchmarkDotNet#1916 my guess is that |
|
cc @radekdoulik |
@LoopedBard3 could you please add link to the log, where this is visible? |
radekdoulik
added a commit
to radekdoulik/BenchmarkDotNet
that referenced
this issue
Mar 2, 2022
Fix dotnet/performance#2250 Commit 80f45ce started using the runtime moniker to decide, which main JS file to use. We use `test-main.js` in net7.0, while we were using `main.js` in older versions. To make that work, let set the right runtime moniker to WasmRuntime. So that later in `Executor::ProcessStartInfo` we select the right file.
adamsitnik
pushed a commit
to dotnet/BenchmarkDotNet
that referenced
this issue
Mar 3, 2022
* [wasm] Set the right runtime moniker Fix dotnet/performance#2250 Commit 80f45ce started using the runtime moniker to decide, which main JS file to use. We use `test-main.js` in net7.0, while we were using `main.js` in older versions. To make that work, let set the right runtime moniker to WasmRuntime. So that later in `Executor::ProcessStartInfo` we select the right file.
|
@LoopedBard3 I assume we need to take another BDN update to pick up dotnet/BenchmarkDotNet#1932 before this can actually be closed? |
|
Yes, it looks this change is a part of version 0.13.1.1706? If so, I can push the new versions and watch for the update in our runs before officially closing this issue. |
|
Updated the BDN. We also found a change to make on our end that helped locally as shown in #2288 so these combined should be the last stuff for the fix. |
|
Mistakenly closed this with the last issue. Although the fixes made it so the tests are running now, but we are running into some sort of timeout: |
|
@radical is looking into this |
|
Looks like #2296 did not fix the issue. Looking more closely at the run itself, it looks like we are setting the build-time to be higher at 3600 (I am assuming seconds). At least based on It does not seem that this is the direct timeout causing the stoppage as that test run starts at 13:51:03 with the stoppage at 14:08:21 (~17 minutes). Is there a different timeout I may be missing? |
|
Looking a little bit above the failure, it looks like the same timeout failure occurs but doesn't cause the exception: |
It seems that a new nuget wasn't generated from that dotnet/BenchmarkDotNet#1936 (dotnet/BenchmarkDotNet#1936 (comment)). An updated nuget reference is needed here to continue rest of the fix. |
|
This is the timeout for the execution of the benchmark running with Shouldn't this timeout be configurable? I haven't been able to reproduce locally though, as I keep running into: |
This was referenced Mar 11, 2022
|
Other parts of the fix: |
this looks like a
It should, but are we sure that it's not a product bug? It's the first time I see anyone having this issue. |
|
This needs to be re-opened:) |
It might be a product bug, so I wanted to try out a longer timeout to see if it passes. |
In a clean This is using a locally setup |
|
oh, and |
|
Reopening as per above |
|
Next PR, towards fixing the AOT runs - dotnet/BenchmarkDotNet#1947 . |
|
Also #2320, and dotnet/runtime#66795 . |
|
And with these, in my manual runs, the AOT run succeeds. |
|
PR to bump BDN in |
|
#2323 and #2320 have both been merged! Is dotnet/runtime#66795 also something that will need to be merged? If so, do we have a manual run of the dotnet-runtime-perf pipeline to make sure it doesn't break other jobs? |
I have kicked off a manual run for that, and I'll report back with the results. |
|
All the wasm benchmark runs are passing now, for |
|
The aot run for the PR failed on one of the partitions while compiling |
|
That |
|
These were the changes between the last working build (after the bump PR was merged), and the breaking one. Though I think it might be due to the issues that I mentioned earlier. |
|
And it's being hit even with the new |
|
Next: #2330 . This will allow the build to complete without any errors, like not being able to copy files because they are in use by another process. |
|
We need the following to fix some issues, and debug random crashes that we are seeing with wasm/aot runs: |
|
Update: The benchmark runs are working pretty well now, except for some sporadic emcc crashes. I will track those in separate issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Wasm Microbenchmarks are still failing after the BDN update. Two spots that pop out are a seeming failure with building, although another build of seemingly the same location succeeds after; and an inability to run a specific 'main.js' file from the browser-wasm/AppBundle folder. Respectively:
and
Upon further investigation, it looks like the main.js file got renamed to test-main.js. Example ls result in AppBundle:
@naricc @SamMonoRT
The text was updated successfully, but these errors were encountered: