-
-
Notifications
You must be signed in to change notification settings - Fork 978
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
Use Pipes for host and benchmark process communication #2092
Conversation
…wo separate pipes accessed via FileStream
The tests are failing because they are relying on writing some custom messages to standard ouput. Since it's not redirected anymore and we don't parse it, they are failing. I'll just rewrite them to write to specific file instead. |
@EgorBo and @stephentoub reported offline an issue where having the std out redirected and BDN reading from it can cause hangs when Repro: using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
[DisassemblyDiagnoser]
[HideColumns("Error", "StdDev", "Median", "RatioSD")]
public partial class Program
{
static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
private static readonly int s_year = DateTime.UtcNow.Year;
[Benchmark]
public int Count()
{
int count = 0;
for (int i = 0; i < 1_000_000; i++)
{
if (s_year == 2021)
{
count++;
}
}
return count;
}
} $env:DOTNET_JitDisasmSummary=1
dotnet run -c Release -f net7.0 --filter * Once I get back to working on this PR I am going to ensure that this scenario works as expected. |
…-blocking way and expose it to the end-users so they can do whatever they want with it
I was able to confirm that this PR solves the problem reported by @stephentoub and @EgorBo Previously it was hanging: Now it works fine: |
…lly, there is no need to write to std out
add an artificial sleep to increase the chance of host process reading all std output.
I've tested locally .NET Framework, .NET (Core), Mono (the old one) and WASM (this was not easy due to missing docs). Everything works fine, merging. |
The deadlock on an unhandled exception occurs here: [Benchmark]
public unsafe void Benchmark_UnhandledException()
{
*(byte*)65536 = 100;
} Slightly relevant here: To proper detect the unhandled exception (#1661) and display its message we should set |
I definitely need to do more testing (Unix, Mono, WASM), but to tell the long story short redirecting STDIN & OUT and using it for process communication has historically proven to be unreliable. We can just use Anonymous Pipes and avoid issues like #2070.
Moreover, by using Pipes we could allow for:
Until I am done with everything this PR is a draft
fixes #2070