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.

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

Merged
merged 23 commits into from
Sep 14, 2022
Merged

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Aug 28, 2022

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

@adamsitnik adamsitnik added this to the v0.13.3 milestone Aug 28, 2022
@adamsitnik adamsitnik changed the title Use Named Pipes for host and benchmark process communication Use Pipes for host and benchmark process communication Aug 29, 2022
@adamsitnik
Copy link
Member Author

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.

@adamsitnik
Copy link
Member Author

adamsitnik commented Sep 6, 2022

@EgorBo and @stephentoub reported offline an issue where having the std out redirected and BDN reading from it can cause hangs when DOTNET_JitDisasmSummary is enabled.

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.

@adamsitnik
Copy link
Member Author

I was able to confirm that this PR solves the problem reported by @stephentoub and @EgorBo

Previously it was hanging:

image

Now it works fine:

image

@adamsitnik adamsitnik marked this pull request as ready for review September 13, 2022 08:12
@adamsitnik
Copy link
Member Author

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.

@YegorStepanov
Copy link
Contributor

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 process.RedirectStandardError=true and read from StandardError, but it will another deadlock (#1631)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BenchmarkDotNet crashing on Linux with DisassemblyDiagnoser
2 participants