From c528ab140b9a0b282b093b4a7e57c70dcc3db332 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 28 Aug 2022 20:12:37 +0200 Subject: [PATCH 01/22] use Named Pipes for inter process communication --- src/BenchmarkDotNet/BenchmarkDotNet.csproj | 3 + src/BenchmarkDotNet/Engines/ConsoleHost.cs | 41 -------- src/BenchmarkDotNet/Engines/IHost.cs | 5 +- src/BenchmarkDotNet/Engines/NamedPipeHost.cs | 98 +++++++++++++++++++ .../Engines/NoAcknowledgementConsoleHost.cs | 14 +-- ...hronousProcessOutputLoggerWithDiagnoser.cs | 37 +++---- .../Portability/RuntimeInformation.cs | 7 +- .../Portability/WindowsSyscallCallHelper.cs | 67 +++++++++++++ src/BenchmarkDotNet/Running/BenchmarkId.cs | 8 +- .../Templates/BenchmarkProgram.txt | 13 +-- .../DotNetCli/DotNetCliCommandExecutor.cs | 10 +- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 7 +- src/BenchmarkDotNet/Toolchains/Executor.cs | 18 ++-- .../Toolchains/InProcess/InProcessHost.cs | 6 +- .../Engine/EngineFactoryTests.cs | 2 +- 15 files changed, 241 insertions(+), 95 deletions(-) delete mode 100644 src/BenchmarkDotNet/Engines/ConsoleHost.cs create mode 100644 src/BenchmarkDotNet/Engines/NamedPipeHost.cs create mode 100644 src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs diff --git a/src/BenchmarkDotNet/BenchmarkDotNet.csproj b/src/BenchmarkDotNet/BenchmarkDotNet.csproj index d2b55cdec8..339aa4ebc4 100644 --- a/src/BenchmarkDotNet/BenchmarkDotNet.csproj +++ b/src/BenchmarkDotNet/BenchmarkDotNet.csproj @@ -54,4 +54,7 @@ + + + diff --git a/src/BenchmarkDotNet/Engines/ConsoleHost.cs b/src/BenchmarkDotNet/Engines/ConsoleHost.cs deleted file mode 100644 index 738555807f..0000000000 --- a/src/BenchmarkDotNet/Engines/ConsoleHost.cs +++ /dev/null @@ -1,41 +0,0 @@ -using System; -using System.IO; -using BenchmarkDotNet.Validators; -using JetBrains.Annotations; - -namespace BenchmarkDotNet.Engines -{ - public sealed class ConsoleHost : IHost - { - private readonly TextWriter outWriter; - private readonly StreamReader inReader; - - public ConsoleHost([NotNull]TextWriter outWriter, [NotNull]StreamReader inReader) - { - this.outWriter = outWriter ?? throw new ArgumentNullException(nameof(outWriter)); - this.inReader = inReader ?? throw new ArgumentNullException(nameof(inReader)); - } - - public void Write(string message) => outWriter.Write(message); - - public void WriteLine() => outWriter.WriteLine(); - - public void WriteLine(string message) => outWriter.WriteLine(message); - - public void SendSignal(HostSignal hostSignal) - { - WriteLine(Engine.Signals.ToMessage(hostSignal)); - - // read the response from Parent process, make the communication blocking - // I did not use Mutexes because they are not supported for Linux/MacOs for .NET Core - // this solution is stupid simple and it works - string acknowledgment = inReader.ReadLine(); - if (acknowledgment != Engine.Signals.Acknowledgment) - throw new NotSupportedException($"Unknown Acknowledgment: {acknowledgment}"); - } - - public void SendError(string message) => outWriter.WriteLine($"{ValidationErrorReporter.ConsoleErrorPrefix} {message}"); - - public void ReportResults(RunResults runResults) => runResults.Print(outWriter); - } -} diff --git a/src/BenchmarkDotNet/Engines/IHost.cs b/src/BenchmarkDotNet/Engines/IHost.cs index 2e6895ebc3..e57ee2172b 100644 --- a/src/BenchmarkDotNet/Engines/IHost.cs +++ b/src/BenchmarkDotNet/Engines/IHost.cs @@ -1,9 +1,10 @@ -using System.Diagnostics.CodeAnalysis; +using System; +using System.Diagnostics.CodeAnalysis; namespace BenchmarkDotNet.Engines { [SuppressMessage("ReSharper", "UnusedMember.Global")] - public interface IHost + public interface IHost : IDisposable { void Write(string message); void WriteLine(); diff --git a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs new file mode 100644 index 0000000000..febcfb658a --- /dev/null +++ b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs @@ -0,0 +1,98 @@ +using BenchmarkDotNet.Portability; +using BenchmarkDotNet.Validators; +using System; +using System.IO; +using System.Text; + +namespace BenchmarkDotNet.Engines +{ + public class NamedPipeHost : IHost + { + internal const string NamedPipeArgument = "--namedPipe"; + internal static readonly Encoding UTF8NoBOM = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + + private readonly FileStream namedPipe; + private readonly StreamWriter outWriter; + private readonly StreamReader inReader; + + public NamedPipeHost(string namedPipePath) + { + namedPipe = OpenFileStream(namedPipePath); + inReader = new StreamReader(namedPipe, UTF8NoBOM, detectEncodingFromByteOrderMarks: false); + outWriter = new StreamWriter(namedPipe, UTF8NoBOM); + // Flush the data to the Stream after each write, otherwise the server will wait for input endlessly! + outWriter.AutoFlush = true; + } + + private FileStream OpenFileStream(string namedPipePath) + { +#if NETSTANDARD + if (RuntimeInformation.IsWindows()) + { + return WindowsSyscallCallHelper.OpenNamedPipe(namedPipePath); + } +#endif + + return new FileStream(namedPipePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + } + + public void Dispose() + { + outWriter.Dispose(); + inReader.Dispose(); + namedPipe.Dispose(); + } + + public void Write(string message) => outWriter.Write(message); + + public void WriteLine() => outWriter.WriteLine(); + + public void WriteLine(string message) => outWriter.WriteLine(message); + + public void SendSignal(HostSignal hostSignal) + { + WriteLine(Engine.Signals.ToMessage(hostSignal)); + + // read the response from Parent process, make the communication blocking + string acknowledgment = inReader.ReadLine(); + if (acknowledgment != Engine.Signals.Acknowledgment) + throw new NotSupportedException($"Unknown Acknowledgment: {acknowledgment}"); + } + + public void SendError(string message) => outWriter.WriteLine($"{ValidationErrorReporter.ConsoleErrorPrefix} {message}"); + + public void ReportResults(RunResults runResults) => runResults.Print(outWriter); + + public static bool TryGetNamedPipePath(string[] args, out string namedPipePath) + { + // This method is invoked at the beginning of every benchmarking process. + // We don't want to JIT any unnecessary .NET methods as this could affect their benchmarks + // Example: Using LINQ here would cause some LINQ methods to be JITed before their first invocation, + // which would make it impossible to measure their jitting time using Job.Dry for example. + + for (int i = 0; i < args.Length; i++) + { + if (args[i] == NamedPipeArgument) + { + namedPipePath = args[i + 1]; // IndexOutOfRangeException means a bug (incomplete data) + return true; + } + } + + namedPipePath = null; + return false; + } + + // logic based on https://github.com/dotnet/runtime/blob/a54a823ece1094dd05b7380614bd43566834a8f9/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs#L154 + internal static (string serverName, string clientPath) GenerateNamedPipeNames() + { + string guid = Guid.NewGuid().ToString("N"); + if (RuntimeInformation.IsWindows()) + { + return (guid, Path.GetFullPath($@"\\.\pipe\{guid}")); + } + + return ($"/tmp/{guid}", $"/tmp/{guid}"); + } + } +} diff --git a/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs b/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs index 4be570e933..47be806ab4 100644 --- a/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs +++ b/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs @@ -1,20 +1,15 @@ using System; using System.IO; using BenchmarkDotNet.Validators; -using JetBrains.Annotations; namespace BenchmarkDotNet.Engines { - /* This class is used by wasm because wasm cannot read from the console. - * This potentially breaks communication with Diagnosers. */ + // this class is used only when somebody manually launches benchmarking .exe without providing named pipe path public sealed class NoAcknowledgementConsoleHost : IHost { private readonly TextWriter outWriter; - public NoAcknowledgementConsoleHost([NotNull]TextWriter outWriter) - { - this.outWriter = outWriter ?? throw new ArgumentNullException(nameof(outWriter)); - } + public NoAcknowledgementConsoleHost() => outWriter = Console.Out; public void Write(string message) => outWriter.Write(message); @@ -27,5 +22,10 @@ public NoAcknowledgementConsoleHost([NotNull]TextWriter outWriter) public void SendError(string message) => outWriter.WriteLine($"{ValidationErrorReporter.ConsoleErrorPrefix} {message}"); public void ReportResults(RunResults runResults) => runResults.Print(outWriter); + + public void Dispose() + { + // do nothing on purpose - there is no point in closing STD OUT + } } } diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs index 6ce058d9d6..6ea3798324 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs @@ -1,9 +1,9 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Diagnostics; +using System.IO; +using System.IO.Pipes; using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Engines; -using BenchmarkDotNet.Environments; using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Loggers @@ -13,19 +13,16 @@ internal class SynchronousProcessOutputLoggerWithDiagnoser private readonly ILogger logger; private readonly Process process; private readonly IDiagnoser diagnoser; + private readonly NamedPipeServerStream namedPipeServer; private readonly DiagnoserActionParameters diagnoserActionParameters; public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process process, IDiagnoser diagnoser, - BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, bool noAcknowledgments) + BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, NamedPipeServerStream namedPipeServer) { - if (!process.StartInfo.RedirectStandardOutput) - throw new NotSupportedException("set RedirectStandardOutput to true first"); - if (!(process.StartInfo.RedirectStandardInput || benchmarkCase.GetRuntime() is WasmRuntime || noAcknowledgments)) - throw new NotSupportedException("set RedirectStandardInput to true first"); - this.logger = logger; this.process = process; this.diagnoser = diagnoser; + this.namedPipeServer = namedPipeServer; diagnoserActionParameters = new DiagnoserActionParameters(process, benchmarkCase, benchmarkId); LinesWithResults = new List(); @@ -38,15 +35,16 @@ public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process proce internal void ProcessInput() { - // Peek -1 or 0 can indicate internal error. - while (!process.StandardOutput.EndOfStream && process.StandardOutput.Peek() > 0) - { - // ReadLine() can actually return string.Empty and null as valid values. - string line = process.StandardOutput.ReadLine(); + namedPipeServer.WaitForConnection(); // wait for the benchmark app to connect first! - if (line == null) - continue; + using StreamReader reader = new (namedPipeServer, NamedPipeHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); + using StreamWriter writer = new (namedPipeServer, NamedPipeHost.UTF8NoBOM, bufferSize: 1); + // Flush the data to the Stream after each write, otherwise the client will wait for input endlessly! + writer.AutoFlush = true; + string line = null; + while ((line = reader.ReadLine()) is not null) + { logger.WriteLine(LogKind.Default, line); if (!line.StartsWith("//")) @@ -57,14 +55,11 @@ internal void ProcessInput() { diagnoser?.Handle(signal, diagnoserActionParameters); - if (process.StartInfo.RedirectStandardInput) - { - process.StandardInput.WriteLine(Engine.Signals.Acknowledgment); - } + writer.WriteLine(Engine.Signals.Acknowledgment); if (signal == HostSignal.AfterAll) { - // we have received the last signal so we can stop reading the output + // we have received the last signal so we can stop reading from the pipe // if the process won't exit after this, its hung and needs to be killed return; } diff --git a/src/BenchmarkDotNet/Portability/RuntimeInformation.cs b/src/BenchmarkDotNet/Portability/RuntimeInformation.cs index 6b049adb33..5b743a3129 100644 --- a/src/BenchmarkDotNet/Portability/RuntimeInformation.cs +++ b/src/BenchmarkDotNet/Portability/RuntimeInformation.cs @@ -76,7 +76,12 @@ private static bool IsAotMethod() #if NET6_0_OR_GREATER [System.Runtime.Versioning.SupportedOSPlatformGuard("windows")] #endif - internal static bool IsWindows() => IsOSPlatform(OSPlatform.Windows); + internal static bool IsWindows() => +#if NET6_0_OR_GREATER + OperatingSystem.IsWindows(); // prefer linker-friendly OperatingSystem APIs +#else + IsOSPlatform(OSPlatform.Windows); +#endif #if NET6_0_OR_GREATER [System.Runtime.Versioning.SupportedOSPlatformGuard("linux")] diff --git a/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs b/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs new file mode 100644 index 0000000000..8df5a773b8 --- /dev/null +++ b/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs @@ -0,0 +1,67 @@ +using Microsoft.Win32.SafeHandles; +using System; +using System.IO; +using System.Runtime.InteropServices; + +namespace BenchmarkDotNet.Portability +{ + internal static class WindowsSyscallCallHelper + { + // Before https://github.com/dotnet/runtime/pull/54676 (.NET 6) + // .NET was not allowing to open named pipes using FileStream(path) + // So here we go, calling sys-call directly... + internal static FileStream OpenNamedPipe(string namePipePath) + { + // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs#L23-L45 + const int access = unchecked((int)0x80000000) | 0x40000000; // Generic Read & Write + SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(HandleInheritability.None); + + SafeFileHandle sfh = CreateFileW(namePipePath, access, FileShare.None, ref secAttrs, FileMode.Open, 0, hTemplateFile: IntPtr.Zero); + if (sfh.IsInvalid) + { + int lastError = Marshal.GetLastWin32Error(); + throw new Exception($"Unable to open Named Pipe {namePipePath}. Last error: {lastError:X}"); + } + + return new FileStream(sfh, FileAccess.ReadWrite); + + [DllImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true)] + static extern SafeFileHandle CreateFileW( + [MarshalAs(UnmanagedType.LPWStr)] string lpFileName, + int dwDesiredAccess, + FileShare dwShareMode, + ref SECURITY_ATTRIBUTES secAttrs, + FileMode dwCreationDisposition, + int dwFlagsAndAttributes, + IntPtr hTemplateFile); + } + + // copied from https://github.com/dotnet/runtime/blob/c59b5171e4fb2b000108bec965f8ce443cb95a12/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Windows.cs#L568 + private static unsafe SECURITY_ATTRIBUTES GetSecAttrs(HandleInheritability inheritability) + { + SECURITY_ATTRIBUTES secAttrs = new SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(SECURITY_ATTRIBUTES), + bInheritHandle = ((inheritability & HandleInheritability.Inheritable) != 0) ? BOOL.TRUE : BOOL.FALSE + }; + + return secAttrs; + } + + // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Windows/Interop.BOOL.cs + internal enum BOOL : int + { + FALSE = 0, + TRUE = 1, + } + + // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SECURITY_ATTRIBUTES.cs + [StructLayout(LayoutKind.Sequential)] + internal struct SECURITY_ATTRIBUTES + { + internal uint nLength; + internal IntPtr lpSecurityDescriptor; + internal BOOL bInheritHandle; + } + } +} diff --git a/src/BenchmarkDotNet/Running/BenchmarkId.cs b/src/BenchmarkDotNet/Running/BenchmarkId.cs index 1d79657bca..6b5c846f20 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkId.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkId.cs @@ -1,4 +1,5 @@ using System; +using BenchmarkDotNet.Engines; using BenchmarkDotNet.Exporters; using BenchmarkDotNet.Extensions; using JetBrains.Annotations; @@ -15,10 +16,15 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) Value = value; FullBenchmarkName = GetBenchmarkName(benchmarkCase); JobId = benchmarkCase.Job.Id; + (NamedPipeServer, NamedPipeClient) = NamedPipeHost.GenerateNamedPipeNames(); } public int Value { get; } + internal string NamedPipeServer { get; } + + private string NamedPipeClient { get; } + private string JobId { get; } private string FullBenchmarkName { get; } @@ -29,7 +35,7 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) public override int GetHashCode() => Value; - public string ToArguments() => $"--benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; + public string ToArguments() => $"{NamedPipeHost.NamedPipeArgument} {NamedPipeClient.Escape()} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; public override string ToString() => Value.ToString(); diff --git a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt index fe53e9e261..ec7f6271b2 100644 --- a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt +++ b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt @@ -24,12 +24,11 @@ namespace BenchmarkDotNet.Autogenerated private static System.Int32 AfterAssemblyLoadingAttached(System.String[] args) { - BenchmarkDotNet.Engines.IHost host = // this variable name is used by CodeGenerator.GetCoreRtSwitch, do NOT change it -#if WASM || NO_ACK - new BenchmarkDotNet.Engines.NoAcknowledgementConsoleHost(System.Console.Out); -#else - new BenchmarkDotNet.Engines.ConsoleHost(System.Console.Out, new System.IO.StreamReader(System.Console.OpenStandardInput(), System.Text.Encoding.UTF8)); -#endif + BenchmarkDotNet.Engines.IHost host; // this variable name is used by CodeGenerator.GetCoreRtSwitch, do NOT change it + if (BenchmarkDotNet.Engines.NamedPipeHost.TryGetNamedPipePath(args, out System.String namedPipePath)) + host = new BenchmarkDotNet.Engines.NamedPipeHost(namedPipePath); + else + host = new BenchmarkDotNet.Engines.NoAcknowledgementConsoleHost(); // the first thing to do is to let diagnosers hook in before anything happens // so all jit-related diagnosers can catch first jit compilation! @@ -81,6 +80,8 @@ namespace BenchmarkDotNet.Autogenerated finally { BenchmarkDotNet.Engines.HostExtensions.AfterAll(host); + + host.Dispose(); } } } diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs index bc196249cc..9d4cefea0c 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs @@ -99,7 +99,7 @@ internal static void LogEnvVars(DotNetCliCommand command) } internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, string workingDirectory, string arguments, - IReadOnlyList environmentVariables = null, bool redirectStandardInput = false, bool redirectStandardError = true) + IReadOnlyList environmentVariables = null, bool redirectStandardInput = false, bool redirectStandardError = true, bool redirectStandardOutput = true) { const string dotnetMultiLevelLookupEnvVarName = "DOTNET_MULTILEVEL_LOOKUP"; @@ -110,12 +110,16 @@ internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, stri Arguments = arguments, UseShellExecute = false, CreateNoWindow = true, - RedirectStandardOutput = true, + RedirectStandardOutput = redirectStandardOutput, RedirectStandardError = redirectStandardError, RedirectStandardInput = redirectStandardInput, - StandardOutputEncoding = Encoding.UTF8, }; + if (redirectStandardOutput) + { + startInfo.StandardOutputEncoding = Encoding.UTF8; + } + if (redirectStandardError) // StandardErrorEncoding is only supported when standard error is redirected { startInfo.StandardErrorEncoding = Encoding.UTF8; diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 282075db8e..882a688124 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -1,6 +1,7 @@ using System; using System.Diagnostics; using System.IO; +using System.IO.Pipes; using BenchmarkDotNet.Characteristics; using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Engines; @@ -68,15 +69,17 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, CustomDotNetCliPath, artifactsPaths.BinariesDirectoryPath, $"{executableName.Escape()} {benchmarkId.ToArguments()}", - redirectStandardInput: !noAcknowledgments, + redirectStandardOutput: false, + redirectStandardInput: false, redirectStandardError: false); // #1629 startInfo.SetEnvironmentVariables(benchmarkCase, resolver); + using (NamedPipeServerStream namedPipeServer = new (benchmarkId.NamedPipeServer, PipeDirection.InOut, maxNumberOfServerInstances: 1)) using (var process = new Process { StartInfo = startInfo }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, noAcknowledgments); + var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, namedPipeServer); logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 6412fd0d29..9bf39da156 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -1,6 +1,7 @@ using System; using System.Diagnostics; using System.IO; +using System.IO.Pipes; using System.Linq; using System.Text; using BenchmarkDotNet.Characteristics; @@ -32,18 +33,19 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) } return Execute(executeParameters.BenchmarkCase, executeParameters.BenchmarkId, executeParameters.Logger, executeParameters.BuildResult.ArtifactsPaths, - args, executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex, executeParameters.BuildResult.NoAcknowledgments); + args, executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex, executeParameters.BenchmarkId.NamedPipeServer); } private ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, - string args, IDiagnoser diagnoser, IResolver resolver, int launchIndex, bool noAcknowledgments) + string args, IDiagnoser diagnoser, IResolver resolver, int launchIndex, string namedPipeServerName) { try { - using (var process = new Process { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver, noAcknowledgments) }) + using (NamedPipeServerStream namedPipeServer = new (namedPipeServerName, PipeDirection.InOut, maxNumberOfServerInstances: 1)) + using (var process = new Process { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver) }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, noAcknowledgments); + var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, namedPipeServer); diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); @@ -89,17 +91,15 @@ private ExecuteResult Execute(Process process, BenchmarkCase benchmarkCase, Sync launchIndex); } - private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsPaths artifactsPaths, - string args, IResolver resolver, bool noAcknowledgments) + private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsPaths artifactsPaths, string args, IResolver resolver) { var start = new ProcessStartInfo { UseShellExecute = false, - RedirectStandardOutput = true, - RedirectStandardInput = !noAcknowledgments, + RedirectStandardOutput = false, + RedirectStandardInput = false, RedirectStandardError = false, // #1629 CreateNoWindow = true, - StandardOutputEncoding = Encoding.UTF8, // #1713 WorkingDirectory = null // by default it's null }; diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/InProcessHost.cs b/src/BenchmarkDotNet/Toolchains/InProcess/InProcessHost.cs index cf73c18794..726d69b2bd 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/InProcessHost.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/InProcessHost.cs @@ -69,7 +69,6 @@ public InProcessHost(BenchmarkCase benchmarkCase, ILogger logger, IDiagnoser dia /// Text to write. public void WriteLine(string message) => logger.WriteLine(message); - /// Sends notification signal to the host. /// The signal to send. public void SendSignal(HostSignal hostSignal) @@ -97,5 +96,10 @@ public void ReportResults(RunResults runResults) logger.Write(w.GetStringBuilder().ToString()); } } + + public void Dispose() + { + // do nothing on purpose + } } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.Tests/Engine/EngineFactoryTests.cs b/tests/BenchmarkDotNet.Tests/Engine/EngineFactoryTests.cs index 89c6fa05bd..19a0e0583f 100644 --- a/tests/BenchmarkDotNet.Tests/Engine/EngineFactoryTests.cs +++ b/tests/BenchmarkDotNet.Tests/Engine/EngineFactoryTests.cs @@ -247,7 +247,7 @@ private EngineParameters CreateEngineParameters(Action mainNoUnroll, Actio Dummy3Action = () => { }, GlobalSetupAction = GlobalSetup, GlobalCleanupAction = GlobalCleanup, - Host = new ConsoleHost(TextWriter.Null, StreamReader.Null), + Host = new NoAcknowledgementConsoleHost(), OverheadActionUnroll = OverheadUnroll, OverheadActionNoUnroll = OverheadNoUnroll, IterationCleanupAction = IterationCleanup, From 2cb73c0af1ed069785053b3fe08d8bec633b7302 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 28 Aug 2022 22:26:47 +0200 Subject: [PATCH 02/22] checking whether diagnoser requires blocking acknowledgments is not needed anymore --- .../ConcurrencyVisualizerProfiler.cs | 2 -- .../EtwDiagnoser.cs | 1 - src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs | 2 -- .../NativeMemoryProfiler.cs | 2 -- src/BenchmarkDotNet/Code/CodeGenerator.cs | 3 --- src/BenchmarkDotNet/Diagnosers/CompositeDiagnoser.cs | 3 --- src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs | 2 -- src/BenchmarkDotNet/Diagnosers/IDiagnoser.cs | 2 -- src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs | 1 - src/BenchmarkDotNet/Diagnosers/ThreadingDiagnoser.cs | 2 -- src/BenchmarkDotNet/Diagnosers/UnresolvedDiagnoser.cs | 1 - .../Disassemblers/DisassemblyDiagnoser.cs | 2 -- src/BenchmarkDotNet/Running/BuildPartition.cs | 5 ----- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 6 ++---- src/BenchmarkDotNet/Toolchains/GeneratorBase.cs | 2 +- .../Toolchains/InProcess.Emit/InProcessEmitBuilder.cs | 3 +-- .../InProcess.Emit/InProcessEmitGenerator.cs | 2 +- .../InProcess.NoEmit/InProcessNoEmitGenerator.cs | 2 +- .../Toolchains/InProcess/InProcessGenerator.cs | 2 +- src/BenchmarkDotNet/Toolchains/Results/BuildResult.cs | 2 +- .../Toolchains/Results/GenerateResult.cs | 10 ++++------ .../BenchmarkDotNet.IntegrationTests/ToolchainTest.cs | 2 +- tests/BenchmarkDotNet.Tests/BuildResultTests.cs | 2 +- tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs | 4 ++-- .../Reports/RatioPrecisionTests.cs | 2 +- tests/BenchmarkDotNet.Tests/Reports/RatioStyleTests.cs | 2 +- tests/BenchmarkDotNet.Tests/Reports/SummaryTests.cs | 2 +- 27 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src/BenchmarkDotNet.Diagnostics.Windows/ConcurrencyVisualizerProfiler.cs b/src/BenchmarkDotNet.Diagnostics.Windows/ConcurrencyVisualizerProfiler.cs index 5a5a76112d..acb6280162 100644 --- a/src/BenchmarkDotNet.Diagnostics.Windows/ConcurrencyVisualizerProfiler.cs +++ b/src/BenchmarkDotNet.Diagnostics.Windows/ConcurrencyVisualizerProfiler.cs @@ -60,8 +60,6 @@ public void DisplayResults(ILogger logger) logger.WriteLineInfo("DO remember that this Diagnoser just tries to mimic the CVCollectionCmd.exe and you need to have Visual Studio with Concurrency Visualizer plugin installed to visualize the data."); } - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => true; - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { etwProfiler.Handle(signal, parameters); diff --git a/src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs b/src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs index b3e62f117d..c52006d7c6 100644 --- a/src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs +++ b/src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs @@ -21,7 +21,6 @@ namespace BenchmarkDotNet.Diagnostics.Windows protected readonly Dictionary BenchmarkToProcess = new Dictionary(); protected readonly ConcurrentDictionary StatsPerProcess = new ConcurrentDictionary(); - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => true; public virtual RunMode GetRunMode(BenchmarkCase benchmarkCase) => RunMode.ExtraRun; public virtual IEnumerable Exporters => Array.Empty(); diff --git a/src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs b/src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs index 6f09ba86ff..6641dd5a45 100644 --- a/src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs +++ b/src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs @@ -60,8 +60,6 @@ public EtwProfiler(EtwProfilerConfig config) public IEnumerable Validate(ValidationParameters validationParameters) => HardwareCounters.Validate(validationParameters, mandatory: false); - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => false; - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { // it's crucial to start the trace before the process starts and stop it after the benchmarked process stops to have all of the necessary events in the trace file! diff --git a/src/BenchmarkDotNet.Diagnostics.Windows/NativeMemoryProfiler.cs b/src/BenchmarkDotNet.Diagnostics.Windows/NativeMemoryProfiler.cs index b05aa2f772..92bc84417c 100644 --- a/src/BenchmarkDotNet.Diagnostics.Windows/NativeMemoryProfiler.cs +++ b/src/BenchmarkDotNet.Diagnostics.Windows/NativeMemoryProfiler.cs @@ -46,8 +46,6 @@ public void DisplayResults(ILogger resultLogger) resultLogger.Write(line.Kind, line.Text); } - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => etwProfiler.RequiresBlockingAcknowledgments(benchmarkCase); - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) => etwProfiler.Handle(signal, parameters); public RunMode GetRunMode(BenchmarkCase benchmarkCase) => etwProfiler.GetRunMode(benchmarkCase); diff --git a/src/BenchmarkDotNet/Code/CodeGenerator.cs b/src/BenchmarkDotNet/Code/CodeGenerator.cs index e60dfe2699..52ae0e5478 100644 --- a/src/BenchmarkDotNet/Code/CodeGenerator.cs +++ b/src/BenchmarkDotNet/Code/CodeGenerator.cs @@ -79,9 +79,6 @@ internal static string Generate(BuildPartition buildPartition) else if (buildPartition.IsWasm) extraDefines.Add("#define WASM"); - if (buildPartition.NoAcknowledgments) - extraDefines.Add("#define NO_ACK"); - string benchmarkProgramContent = new SmartStringBuilder(ResourceHelper.LoadTemplate("BenchmarkProgram.txt")) .Replace("$ShadowCopyDefines$", useShadowCopy ? "#define SHADOWCOPY" : null).Replace("$ShadowCopyFolderPath$", shadowCopyFolderPath) .Replace("$ExtraDefines$", string.Join(Environment.NewLine, extraDefines)) diff --git a/src/BenchmarkDotNet/Diagnosers/CompositeDiagnoser.cs b/src/BenchmarkDotNet/Diagnosers/CompositeDiagnoser.cs index 202822af77..66675f1cb9 100644 --- a/src/BenchmarkDotNet/Diagnosers/CompositeDiagnoser.cs +++ b/src/BenchmarkDotNet/Diagnosers/CompositeDiagnoser.cs @@ -31,9 +31,6 @@ public IEnumerable Exporters public IEnumerable Analysers => diagnosers.SelectMany(diagnoser => diagnoser.Analysers); - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) - => diagnosers.Any(diagnoser => diagnoser.RequiresBlockingAcknowledgments(benchmarkCase)); - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { foreach (var diagnoser in diagnosers) diff --git a/src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs b/src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs index e1e655cd86..211d4a06e2 100644 --- a/src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs +++ b/src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs @@ -69,8 +69,6 @@ public IEnumerable Validate(ValidationParameters validationPara } } - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => true; - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { if (signal != HostSignal.BeforeAnythingElse) diff --git a/src/BenchmarkDotNet/Diagnosers/IDiagnoser.cs b/src/BenchmarkDotNet/Diagnosers/IDiagnoser.cs index 8045b8e7fa..355dd852e0 100644 --- a/src/BenchmarkDotNet/Diagnosers/IDiagnoser.cs +++ b/src/BenchmarkDotNet/Diagnosers/IDiagnoser.cs @@ -20,8 +20,6 @@ public interface IDiagnoser RunMode GetRunMode(BenchmarkCase benchmarkCase); - bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase); - void Handle(HostSignal signal, DiagnoserActionParameters parameters); IEnumerable ProcessResults(DiagnoserResults results); diff --git a/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs b/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs index 862e6cc041..b6c2eb12bb 100644 --- a/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs +++ b/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs @@ -30,7 +30,6 @@ public void DisplayResults(ILogger logger) { } public IEnumerable Validate(ValidationParameters validationParameters) => Array.Empty(); // the action takes places in other process, and the values are gathered by Engine - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => false; public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { } public IEnumerable ProcessResults(DiagnoserResults diagnoserResults) diff --git a/src/BenchmarkDotNet/Diagnosers/ThreadingDiagnoser.cs b/src/BenchmarkDotNet/Diagnosers/ThreadingDiagnoser.cs index a1997c23ff..cf19cba1e1 100644 --- a/src/BenchmarkDotNet/Diagnosers/ThreadingDiagnoser.cs +++ b/src/BenchmarkDotNet/Diagnosers/ThreadingDiagnoser.cs @@ -29,8 +29,6 @@ public void DisplayResults(ILogger logger) { } public RunMode GetRunMode(BenchmarkCase benchmarkCase) => RunMode.NoOverhead; - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => false; - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { } public IEnumerable ProcessResults(DiagnoserResults results) diff --git a/src/BenchmarkDotNet/Diagnosers/UnresolvedDiagnoser.cs b/src/BenchmarkDotNet/Diagnosers/UnresolvedDiagnoser.cs index 6db34ef072..7ec0498c34 100644 --- a/src/BenchmarkDotNet/Diagnosers/UnresolvedDiagnoser.cs +++ b/src/BenchmarkDotNet/Diagnosers/UnresolvedDiagnoser.cs @@ -22,7 +22,6 @@ public class UnresolvedDiagnoser : IDiagnoser public IEnumerable Ids => Array.Empty(); public IEnumerable Exporters => Array.Empty(); public IEnumerable Analysers => Array.Empty(); - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => false; public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { } public IEnumerable ProcessResults(DiagnoserResults _) => Array.Empty(); diff --git a/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs b/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs index 6b094ae4cd..376788ffae 100644 --- a/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs +++ b/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs @@ -65,8 +65,6 @@ public RunMode GetRunMode(BenchmarkCase benchmarkCase) return RunMode.None; } - public bool RequiresBlockingAcknowledgments(BenchmarkCase benchmarkCase) => GetRunMode(benchmarkCase) != RunMode.None; - public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { var benchmark = parameters.BenchmarkCase; diff --git a/src/BenchmarkDotNet/Running/BuildPartition.cs b/src/BenchmarkDotNet/Running/BuildPartition.cs index 3a98ea5a56..5275acd61b 100644 --- a/src/BenchmarkDotNet/Running/BuildPartition.cs +++ b/src/BenchmarkDotNet/Running/BuildPartition.cs @@ -68,11 +68,6 @@ public BuildPartition(BenchmarkBuildInfo[] benchmarks, IResolver resolver) public bool GenerateMSBuildBinLog { get; } - public bool NoAcknowledgments - => !Benchmarks - .Any(bennchmark => bennchmark.Config.GetDiagnosers() - .Any(diagnoser => diagnoser.RequiresBlockingAcknowledgments(bennchmark.BenchmarkCase))); - public override string ToString() => RepresentativeBenchmarkCase.Job.DisplayInfo; private static string GetResolvedAssemblyLocation(Assembly assembly) => diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 882a688124..b2a4768e28 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -44,8 +44,7 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) executeParameters.Diagnoser, Path.GetFileName(executeParameters.BuildResult.ArtifactsPaths.ExecutablePath), executeParameters.Resolver, - executeParameters.LaunchIndex, - executeParameters.BuildResult.NoAcknowledgments); + executeParameters.LaunchIndex); } finally { @@ -62,8 +61,7 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, IDiagnoser diagnoser, string executableName, IResolver resolver, - int launchIndex, - bool noAcknowledgments) + int launchIndex) { var startInfo = DotNetCliCommandExecutor.BuildStartInfo( CustomDotNetCliPath, diff --git a/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs b/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs index 81ebf2100a..0d7ab30d7d 100644 --- a/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs +++ b/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs @@ -29,7 +29,7 @@ public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger log GenerateProject(buildPartition, artifactsPaths, logger); GenerateBuildScript(buildPartition, artifactsPaths); - return GenerateResult.Success(artifactsPaths, GetArtifactsToCleanup(artifactsPaths), buildPartition.NoAcknowledgments); + return GenerateResult.Success(artifactsPaths, GetArtifactsToCleanup(artifactsPaths)); } catch (Exception ex) { diff --git a/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitBuilder.cs b/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitBuilder.cs index 6381f709f7..f46f5c99b6 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitBuilder.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitBuilder.cs @@ -29,8 +29,7 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart return BuildResult.Success( GenerateResult.Success( new InProcessEmitArtifactsPath(assembly, generateResult.ArtifactsPaths), - generateResult.ArtifactsToCleanup, - generateResult.NoAcknowledgments)); + generateResult.ArtifactsToCleanup)); } } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitGenerator.cs b/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitGenerator.cs index 8035f7854d..c169c91903 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess.Emit/InProcessEmitGenerator.cs @@ -20,7 +20,7 @@ public GenerateResult GenerateProject( { artifactsPaths = GetArtifactsPaths(buildPartition, rootArtifactsFolderPath); - return GenerateResult.Success(artifactsPaths, new List(), buildPartition.NoAcknowledgments); + return GenerateResult.Success(artifactsPaths, new List()); } catch (Exception ex) { diff --git a/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitGenerator.cs b/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitGenerator.cs index 91ec5b89fe..cd1b24dbf8 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitGenerator.cs @@ -13,6 +13,6 @@ public class InProcessNoEmitGenerator : IGenerator { /// returns a success public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger logger, string rootArtifactsFolderPath) - => GenerateResult.Success(null, Array.Empty(), buildPartition.NoAcknowledgments); + => GenerateResult.Success(null, Array.Empty()); } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/InProcessGenerator.cs b/src/BenchmarkDotNet/Toolchains/InProcess/InProcessGenerator.cs index 8a062e5ab7..ca9803df55 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/InProcessGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/InProcessGenerator.cs @@ -13,6 +13,6 @@ public class InProcessGenerator : IGenerator { /// returns a success public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger logger, string rootArtifactsFolderPath) - => GenerateResult.Success(null, Array.Empty(), buildPartition.NoAcknowledgments); + => GenerateResult.Success(null, Array.Empty()); } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Toolchains/Results/BuildResult.cs b/src/BenchmarkDotNet/Toolchains/Results/BuildResult.cs index ac1a460b3c..cf2b5729e5 100644 --- a/src/BenchmarkDotNet/Toolchains/Results/BuildResult.cs +++ b/src/BenchmarkDotNet/Toolchains/Results/BuildResult.cs @@ -11,7 +11,7 @@ public class BuildResult : GenerateResult public string ErrorMessage { get; } private BuildResult(GenerateResult generateResult, bool isBuildSuccess, string errorMessage) - : base(generateResult.ArtifactsPaths, generateResult.IsGenerateSuccess, generateResult.GenerateException, generateResult.ArtifactsToCleanup, generateResult.NoAcknowledgments) + : base(generateResult.ArtifactsPaths, generateResult.IsGenerateSuccess, generateResult.GenerateException, generateResult.ArtifactsToCleanup) { IsBuildSuccess = isBuildSuccess; ErrorMessage = errorMessage; diff --git a/src/BenchmarkDotNet/Toolchains/Results/GenerateResult.cs b/src/BenchmarkDotNet/Toolchains/Results/GenerateResult.cs index 23d4affa06..b6de670b8d 100644 --- a/src/BenchmarkDotNet/Toolchains/Results/GenerateResult.cs +++ b/src/BenchmarkDotNet/Toolchains/Results/GenerateResult.cs @@ -9,23 +9,21 @@ public class GenerateResult public bool IsGenerateSuccess { get; } public Exception GenerateException { get; } public IReadOnlyCollection ArtifactsToCleanup { get; } - public bool NoAcknowledgments { get; } public GenerateResult(ArtifactsPaths artifactsPaths, bool isGenerateSuccess, Exception generateException, - IReadOnlyCollection artifactsToCleanup, bool noAcknowledgments) + IReadOnlyCollection artifactsToCleanup) { ArtifactsPaths = artifactsPaths; IsGenerateSuccess = isGenerateSuccess; GenerateException = generateException; ArtifactsToCleanup = artifactsToCleanup; - NoAcknowledgments = noAcknowledgments; } - public static GenerateResult Success(ArtifactsPaths artifactsPaths, IReadOnlyCollection artifactsToCleanup, bool noAcknowledgments) - => new GenerateResult(artifactsPaths, true, null, artifactsToCleanup, noAcknowledgments); + public static GenerateResult Success(ArtifactsPaths artifactsPaths, IReadOnlyCollection artifactsToCleanup) + => new GenerateResult(artifactsPaths, true, null, artifactsToCleanup); public static GenerateResult Failure(ArtifactsPaths artifactsPaths, IReadOnlyCollection artifactsToCleanup, Exception exception = null) - => new GenerateResult(artifactsPaths, false, exception, artifactsToCleanup, false); + => new GenerateResult(artifactsPaths, false, exception, artifactsToCleanup); public override string ToString() => "GenerateResult: " + (IsGenerateSuccess ? "Success" : "Fail"); } diff --git a/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs index 262b0facac..38cac6dfb5 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs @@ -25,7 +25,7 @@ public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger log { logger.WriteLine("Generating"); Done = true; - return new GenerateResult(null, true, null, Array.Empty(), false); + return new GenerateResult(null, true, null, Array.Empty()); } } diff --git a/tests/BenchmarkDotNet.Tests/BuildResultTests.cs b/tests/BenchmarkDotNet.Tests/BuildResultTests.cs index 227cc3ae19..978da2a516 100644 --- a/tests/BenchmarkDotNet.Tests/BuildResultTests.cs +++ b/tests/BenchmarkDotNet.Tests/BuildResultTests.cs @@ -54,7 +54,7 @@ public void UnknownMSBuildErrorsAreNotTranslatedToMoreUserFriendlyVersions() [AssertionMethod] private void Verify(string msbuildError, bool expectedResult, string expectedReason) { - var sut = BuildResult.Failure(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false), msbuildError); + var sut = BuildResult.Failure(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty()), msbuildError); Assert.Equal(expectedResult, sut.TryToExplainFailureReason(out string reason)); Assert.Equal(expectedReason, reason); diff --git a/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs b/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs index aca1026861..27216d830f 100644 --- a/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs +++ b/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs @@ -67,7 +67,7 @@ private static BenchmarkCase[] CreateBenchmarks(IConfig config) private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, int n, double nanoseconds) { - var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false)); + var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty())); var measurements = Enumerable.Range(0, n) .Select(index => new Measurement(1, IterationMode.Workload, IterationStage.Result, index + 1, 1, nanoseconds + index).ToString()) .ToList(); @@ -77,7 +77,7 @@ private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, int n, private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, bool hugeSd, Metric[] metrics) { - var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false)); + var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty())); bool isFoo = benchmarkCase.Descriptor.WorkloadMethodDisplayInfo == "Foo"; bool isBar = benchmarkCase.Descriptor.WorkloadMethodDisplayInfo == "Bar"; var measurements = new List diff --git a/tests/BenchmarkDotNet.Tests/Reports/RatioPrecisionTests.cs b/tests/BenchmarkDotNet.Tests/Reports/RatioPrecisionTests.cs index 6415157804..27f2db779c 100644 --- a/tests/BenchmarkDotNet.Tests/Reports/RatioPrecisionTests.cs +++ b/tests/BenchmarkDotNet.Tests/Reports/RatioPrecisionTests.cs @@ -81,7 +81,7 @@ private Summary CreateSummary(int[] values) private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, int measurementValue) { - var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false)); + var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty())); var measurements = new List { new Measurement(1, IterationMode.Workload, IterationStage.Result, 1, 1, measurementValue), diff --git a/tests/BenchmarkDotNet.Tests/Reports/RatioStyleTests.cs b/tests/BenchmarkDotNet.Tests/Reports/RatioStyleTests.cs index 3b609eb636..c6e2f72191 100644 --- a/tests/BenchmarkDotNet.Tests/Reports/RatioStyleTests.cs +++ b/tests/BenchmarkDotNet.Tests/Reports/RatioStyleTests.cs @@ -102,7 +102,7 @@ private Summary CreateSummary(int[] values, RatioStyle ratioStyle, int noise) private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, int measurementValue, int noise) { - var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false)); + var buildResult = BuildResult.Success(GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty())); var measurements = new List { new Measurement(1, IterationMode.Workload, IterationStage.Result, 1, 1, measurementValue), diff --git a/tests/BenchmarkDotNet.Tests/Reports/SummaryTests.cs b/tests/BenchmarkDotNet.Tests/Reports/SummaryTests.cs index 369d1929c2..d0f8452e35 100644 --- a/tests/BenchmarkDotNet.Tests/Reports/SummaryTests.cs +++ b/tests/BenchmarkDotNet.Tests/Reports/SummaryTests.cs @@ -64,7 +64,7 @@ private static BenchmarkReport CreateFailureReport(BenchmarkCase benchmark) private static BenchmarkReport CreateSuccessReport(BenchmarkCase benchmark) { - GenerateResult generateResult = GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty(), false); + GenerateResult generateResult = GenerateResult.Success(ArtifactsPaths.Empty, Array.Empty()); BuildResult buildResult = BuildResult.Success(generateResult); var metrics = new[] { new Metric(new FakeMetricDescriptor(), Math.E) }; return new BenchmarkReport(true, benchmark, generateResult, buildResult, Array.Empty(), metrics); From 3f54c1adee39f8e3d786e2547971d0c6c2b3293c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 29 Aug 2022 09:06:27 +0200 Subject: [PATCH 03/22] use NamedPipeClientStream on Unix --- src/BenchmarkDotNet/Engines/NamedPipeHost.cs | 21 +++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs index febcfb658a..e29d83f76e 100644 --- a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs +++ b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs @@ -2,6 +2,7 @@ using BenchmarkDotNet.Validators; using System; using System.IO; +using System.IO.Pipes; using System.Text; namespace BenchmarkDotNet.Engines @@ -11,29 +12,35 @@ public class NamedPipeHost : IHost internal const string NamedPipeArgument = "--namedPipe"; internal static readonly Encoding UTF8NoBOM = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); - private readonly FileStream namedPipe; + private readonly Stream namedPipe; private readonly StreamWriter outWriter; private readonly StreamReader inReader; public NamedPipeHost(string namedPipePath) { - namedPipe = OpenFileStream(namedPipePath); + namedPipe = OpenNamedPipe(namedPipePath); inReader = new StreamReader(namedPipe, UTF8NoBOM, detectEncodingFromByteOrderMarks: false); outWriter = new StreamWriter(namedPipe, UTF8NoBOM); // Flush the data to the Stream after each write, otherwise the server will wait for input endlessly! outWriter.AutoFlush = true; } - private FileStream OpenFileStream(string namedPipePath) + private Stream OpenNamedPipe(string namedPipePath) { -#if NETSTANDARD if (RuntimeInformation.IsWindows()) { +#if NET6_0_OR_GREATER + return new FileStream(namedPipePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); +#else return WindowsSyscallCallHelper.OpenNamedPipe(namedPipePath); - } #endif - - return new FileStream(namedPipePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + } + else + { + NamedPipeClientStream client = new (namedPipePath); + client.Connect(timeout: (int)TimeSpan.FromSeconds(10).TotalMilliseconds); + return client; + } } public void Dispose() From 54bfb75c81c329eeb41d19afaf83de2cf21b8833 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 29 Aug 2022 10:43:10 +0200 Subject: [PATCH 04/22] dont use NamedPipeClient as its unsupported on WASM, use mkfifo and two separate pipes accessed via FileStream --- src/BenchmarkDotNet/Engines/NamedPipeHost.cs | 67 ++++++++++++------- ...hronousProcessOutputLoggerWithDiagnoser.cs | 23 ++++--- .../Portability/WindowsSyscallCallHelper.cs | 6 +- src/BenchmarkDotNet/Running/BenchmarkId.cs | 13 ++-- .../Templates/BenchmarkProgram.txt | 4 +- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 5 +- src/BenchmarkDotNet/Toolchains/Executor.cs | 9 +-- 7 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs index e29d83f76e..e5afaee6e4 100644 --- a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs +++ b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs @@ -3,7 +3,10 @@ using System; using System.IO; using System.IO.Pipes; +using System.Runtime.InteropServices; using System.Text; +using JetBrains.Annotations; +using RuntimeInformation = BenchmarkDotNet.Portability.RuntimeInformation; namespace BenchmarkDotNet.Engines { @@ -12,42 +15,34 @@ public class NamedPipeHost : IHost internal const string NamedPipeArgument = "--namedPipe"; internal static readonly Encoding UTF8NoBOM = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); - private readonly Stream namedPipe; private readonly StreamWriter outWriter; private readonly StreamReader inReader; - public NamedPipeHost(string namedPipePath) + public NamedPipeHost(string namedPipeWritePath, string namedPipeReadPath) { - namedPipe = OpenNamedPipe(namedPipePath); - inReader = new StreamReader(namedPipe, UTF8NoBOM, detectEncodingFromByteOrderMarks: false); - outWriter = new StreamWriter(namedPipe, UTF8NoBOM); + outWriter = new StreamWriter(OpenNamedPipe(namedPipeWritePath, FileAccess.Write), UTF8NoBOM); // Flush the data to the Stream after each write, otherwise the server will wait for input endlessly! outWriter.AutoFlush = true; + inReader = new StreamReader(OpenNamedPipe(namedPipeReadPath, FileAccess.Read), UTF8NoBOM, detectEncodingFromByteOrderMarks: false); } - private Stream OpenNamedPipe(string namedPipePath) + private Stream OpenNamedPipe(string namedPipePath, FileAccess access) { +#if NETSTANDARD if (RuntimeInformation.IsWindows()) { -#if NET6_0_OR_GREATER - return new FileStream(namedPipePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); -#else - return WindowsSyscallCallHelper.OpenNamedPipe(namedPipePath); -#endif - } - else - { - NamedPipeClientStream client = new (namedPipePath); - client.Connect(timeout: (int)TimeSpan.FromSeconds(10).TotalMilliseconds); - return client; + return WindowsSyscallCallHelper.OpenNamedPipe(namedPipePath, access); } +#endif + + FileShare share = RuntimeInformation.IsWindows() ? FileShare.None : FileShare.ReadWrite; + return new FileStream(namedPipePath, FileMode.Open, access, share); } public void Dispose() { outWriter.Dispose(); inReader.Dispose(); - namedPipe.Dispose(); } public void Write(string message) => outWriter.Write(message); @@ -70,23 +65,20 @@ public void SendSignal(HostSignal hostSignal) public void ReportResults(RunResults runResults) => runResults.Print(outWriter); - public static bool TryGetNamedPipePath(string[] args, out string namedPipePath) + [PublicAPI] // called from generated code + public static bool TryGetNamedPipePath(string[] args, out string namedPipeWritePath, out string namedPipeReadPath) { - // This method is invoked at the beginning of every benchmarking process. - // We don't want to JIT any unnecessary .NET methods as this could affect their benchmarks - // Example: Using LINQ here would cause some LINQ methods to be JITed before their first invocation, - // which would make it impossible to measure their jitting time using Job.Dry for example. - for (int i = 0; i < args.Length; i++) { if (args[i] == NamedPipeArgument) { - namedPipePath = args[i + 1]; // IndexOutOfRangeException means a bug (incomplete data) + namedPipeWritePath = args[i + 1]; // IndexOutOfRangeException means a bug (incomplete data) + namedPipeReadPath = args[i + 2]; return true; } } - namedPipePath = null; + namedPipeWritePath = namedPipeReadPath = null; return false; } @@ -101,5 +93,28 @@ internal static (string serverName, string clientPath) GenerateNamedPipeNames() return ($"/tmp/{guid}", $"/tmp/{guid}"); } + + internal static Stream CreateServer(string serverName, string filePath, PipeDirection pipeDirection) + { + if (RuntimeInformation.IsWindows()) + { + return new NamedPipeServerStream(serverName, pipeDirection, maxNumberOfServerInstances: 1); + } + else + { + // mkfifo is not supported by WASM, but it's not a problem, as for WASM the host process is always .NET + // and the benchmark process is actual WASM (it just opens the file using FileStream) + int fd = mkfifo(filePath, 438); // 438 stands for 666 in octal + if (fd == -1) + { + throw new Exception("Unable to create named pipe"); + } + + return new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite, bufferSize: 1); + } + + [DllImport("libc", SetLastError = true)] + static extern int mkfifo(string path, int mode); + } } } diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs index 6ea3798324..9a24c7c4be 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs @@ -11,18 +11,17 @@ namespace BenchmarkDotNet.Loggers internal class SynchronousProcessOutputLoggerWithDiagnoser { private readonly ILogger logger; - private readonly Process process; private readonly IDiagnoser diagnoser; - private readonly NamedPipeServerStream namedPipeServer; + private readonly Stream inputFromBenchmark, acknowledgments; private readonly DiagnoserActionParameters diagnoserActionParameters; public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process process, IDiagnoser diagnoser, - BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, NamedPipeServerStream namedPipeServer) + BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, Stream inputFromBenchmark, Stream acknowledgments) { this.logger = logger; - this.process = process; this.diagnoser = diagnoser; - this.namedPipeServer = namedPipeServer; + this.inputFromBenchmark = inputFromBenchmark; + this.acknowledgments = acknowledgments; diagnoserActionParameters = new DiagnoserActionParameters(process, benchmarkCase, benchmarkId); LinesWithResults = new List(); @@ -35,10 +34,13 @@ public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process proce internal void ProcessInput() { - namedPipeServer.WaitForConnection(); // wait for the benchmark app to connect first! + if (inputFromBenchmark is NamedPipeServerStream windowsRead) + { + windowsRead.WaitForConnection(); // wait for the benchmark app to connect first! + } - using StreamReader reader = new (namedPipeServer, NamedPipeHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); - using StreamWriter writer = new (namedPipeServer, NamedPipeHost.UTF8NoBOM, bufferSize: 1); + using StreamReader reader = new (inputFromBenchmark, NamedPipeHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); + using StreamWriter writer = new (acknowledgments, NamedPipeHost.UTF8NoBOM, bufferSize: 1); // Flush the data to the Stream after each write, otherwise the client will wait for input endlessly! writer.AutoFlush = true; string line = null; @@ -55,6 +57,11 @@ internal void ProcessInput() { diagnoser?.Handle(signal, diagnoserActionParameters); + if (acknowledgments is NamedPipeServerStream windowsWrite) + { + windowsWrite.WaitForConnection(); // wait for the benchmark app to connect first! + } + writer.WriteLine(Engine.Signals.Acknowledgment); if (signal == HostSignal.AfterAll) diff --git a/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs b/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs index 8df5a773b8..3953002247 100644 --- a/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs +++ b/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs @@ -10,10 +10,10 @@ internal static class WindowsSyscallCallHelper // Before https://github.com/dotnet/runtime/pull/54676 (.NET 6) // .NET was not allowing to open named pipes using FileStream(path) // So here we go, calling sys-call directly... - internal static FileStream OpenNamedPipe(string namePipePath) + internal static FileStream OpenNamedPipe(string namePipePath, FileAccess fileAccess) { // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs#L23-L45 - const int access = unchecked((int)0x80000000) | 0x40000000; // Generic Read & Write + int access = fileAccess == FileAccess.Read ? unchecked((int)0x80000000) : 0x40000000; // Generic Read & Write SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(HandleInheritability.None); SafeFileHandle sfh = CreateFileW(namePipePath, access, FileShare.None, ref secAttrs, FileMode.Open, 0, hTemplateFile: IntPtr.Zero); @@ -23,7 +23,7 @@ internal static FileStream OpenNamedPipe(string namePipePath) throw new Exception($"Unable to open Named Pipe {namePipePath}. Last error: {lastError:X}"); } - return new FileStream(sfh, FileAccess.ReadWrite); + return new FileStream(sfh, fileAccess); [DllImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true)] static extern SafeFileHandle CreateFileW( diff --git a/src/BenchmarkDotNet/Running/BenchmarkId.cs b/src/BenchmarkDotNet/Running/BenchmarkId.cs index 6b5c846f20..22b27fb055 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkId.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkId.cs @@ -16,14 +16,19 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) Value = value; FullBenchmarkName = GetBenchmarkName(benchmarkCase); JobId = benchmarkCase.Job.Id; - (NamedPipeServer, NamedPipeClient) = NamedPipeHost.GenerateNamedPipeNames(); + (NamedPipeServer_CTS, NamedPipeClient_CTS) = NamedPipeHost.GenerateNamedPipeNames(); + (NamedPipeServer_STC, NamedPipeClient_STC) = NamedPipeHost.GenerateNamedPipeNames(); } public int Value { get; } - internal string NamedPipeServer { get; } + internal string NamedPipeServer_CTS { get; } - private string NamedPipeClient { get; } + internal string NamedPipeClient_CTS { get; } + + internal string NamedPipeServer_STC { get; } + + internal string NamedPipeClient_STC { get; } private string JobId { get; } @@ -35,7 +40,7 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) public override int GetHashCode() => Value; - public string ToArguments() => $"{NamedPipeHost.NamedPipeArgument} {NamedPipeClient.Escape()} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; + public string ToArguments() => $"{NamedPipeHost.NamedPipeArgument} {NamedPipeClient_CTS.Escape()} {NamedPipeClient_STC.Escape()} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; public override string ToString() => Value.ToString(); diff --git a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt index ec7f6271b2..16804411e2 100644 --- a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt +++ b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt @@ -25,8 +25,8 @@ namespace BenchmarkDotNet.Autogenerated private static System.Int32 AfterAssemblyLoadingAttached(System.String[] args) { BenchmarkDotNet.Engines.IHost host; // this variable name is used by CodeGenerator.GetCoreRtSwitch, do NOT change it - if (BenchmarkDotNet.Engines.NamedPipeHost.TryGetNamedPipePath(args, out System.String namedPipePath)) - host = new BenchmarkDotNet.Engines.NamedPipeHost(namedPipePath); + if (BenchmarkDotNet.Engines.NamedPipeHost.TryGetNamedPipePath(args, out System.String namedPipeWritePath, out System.String namedPipeReadPath)) + host = new BenchmarkDotNet.Engines.NamedPipeHost(namedPipeWritePath, namedPipeReadPath); else host = new BenchmarkDotNet.Engines.NoAcknowledgementConsoleHost(); diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index b2a4768e28..3a86daa912 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -73,11 +73,12 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, startInfo.SetEnvironmentVariables(benchmarkCase, resolver); - using (NamedPipeServerStream namedPipeServer = new (benchmarkId.NamedPipeServer, PipeDirection.InOut, maxNumberOfServerInstances: 1)) + using (Stream inputFromBenchmark = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_CTS, benchmarkId.NamedPipeClient_CTS, PipeDirection.In)) + using (Stream acknowledgments = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_STC, benchmarkId.NamedPipeClient_STC, PipeDirection.Out)) using (var process = new Process { StartInfo = startInfo }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, namedPipeServer); + var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 9bf39da156..27b73960ce 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -33,19 +33,20 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) } return Execute(executeParameters.BenchmarkCase, executeParameters.BenchmarkId, executeParameters.Logger, executeParameters.BuildResult.ArtifactsPaths, - args, executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex, executeParameters.BenchmarkId.NamedPipeServer); + args, executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex); } private ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, - string args, IDiagnoser diagnoser, IResolver resolver, int launchIndex, string namedPipeServerName) + string args, IDiagnoser diagnoser, IResolver resolver, int launchIndex) { try { - using (NamedPipeServerStream namedPipeServer = new (namedPipeServerName, PipeDirection.InOut, maxNumberOfServerInstances: 1)) + using (Stream inputFromBenchmark = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_CTS, benchmarkId.NamedPipeClient_CTS, PipeDirection.In)) + using (Stream acknowledgments = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_STC, benchmarkId.NamedPipeClient_STC, PipeDirection.Out)) using (var process = new Process { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver) }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, namedPipeServer); + var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); From 4f746bfa0c53b774ee3cb9a375c13a3d974492d1 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 29 Aug 2022 11:07:44 +0200 Subject: [PATCH 05/22] fix Windows implementation --- .../SynchronousProcessOutputLoggerWithDiagnoser.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs index 9a24c7c4be..a959dcc4ec 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs @@ -4,6 +4,7 @@ using System.IO.Pipes; using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Engines; +using BenchmarkDotNet.Portability; using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Loggers @@ -34,9 +35,11 @@ public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process proce internal void ProcessInput() { - if (inputFromBenchmark is NamedPipeServerStream windowsRead) + if (RuntimeInformation.IsWindows()) { - windowsRead.WaitForConnection(); // wait for the benchmark app to connect first! + // wait for the benchmark app to connect first! + ((NamedPipeServerStream)inputFromBenchmark).WaitForConnection(); + ((NamedPipeServerStream)acknowledgments).WaitForConnection(); } using StreamReader reader = new (inputFromBenchmark, NamedPipeHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); @@ -57,11 +60,6 @@ internal void ProcessInput() { diagnoser?.Handle(signal, diagnoserActionParameters); - if (acknowledgments is NamedPipeServerStream windowsWrite) - { - windowsWrite.WaitForConnection(); // wait for the benchmark app to connect first! - } - writer.WriteLine(Engine.Signals.Acknowledgment); if (signal == HostSignal.AfterAll) From f4b5b781495c372dbd059ee9d6264d75886fc216 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 29 Aug 2022 11:24:13 +0200 Subject: [PATCH 06/22] use Anonymous Pipes --- src/BenchmarkDotNet/BenchmarkDotNet.csproj | 3 - src/BenchmarkDotNet/Engines/NamedPipeHost.cs | 78 ++++--------------- .../Engines/NoAcknowledgementConsoleHost.cs | 2 +- ...hronousProcessOutputLoggerWithDiagnoser.cs | 16 +--- .../Portability/WindowsSyscallCallHelper.cs | 67 ---------------- src/BenchmarkDotNet/Running/BenchmarkId.cs | 12 +-- .../Templates/BenchmarkProgram.txt | 4 +- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 7 +- src/BenchmarkDotNet/Toolchains/Executor.cs | 12 +-- 9 files changed, 34 insertions(+), 167 deletions(-) delete mode 100644 src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs diff --git a/src/BenchmarkDotNet/BenchmarkDotNet.csproj b/src/BenchmarkDotNet/BenchmarkDotNet.csproj index 339aa4ebc4..d2b55cdec8 100644 --- a/src/BenchmarkDotNet/BenchmarkDotNet.csproj +++ b/src/BenchmarkDotNet/BenchmarkDotNet.csproj @@ -54,7 +54,4 @@ - - - diff --git a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs index e5afaee6e4..f71aefaefc 100644 --- a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs +++ b/src/BenchmarkDotNet/Engines/NamedPipeHost.cs @@ -1,43 +1,30 @@ -using BenchmarkDotNet.Portability; -using BenchmarkDotNet.Validators; +using BenchmarkDotNet.Validators; using System; using System.IO; -using System.IO.Pipes; -using System.Runtime.InteropServices; using System.Text; +using Microsoft.Win32.SafeHandles; using JetBrains.Annotations; -using RuntimeInformation = BenchmarkDotNet.Portability.RuntimeInformation; namespace BenchmarkDotNet.Engines { - public class NamedPipeHost : IHost + public class AnonymousPipesHost : IHost { - internal const string NamedPipeArgument = "--namedPipe"; + internal const string AnonymousPipesDescriptors = "--anonymousPipes"; internal static readonly Encoding UTF8NoBOM = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); private readonly StreamWriter outWriter; private readonly StreamReader inReader; - public NamedPipeHost(string namedPipeWritePath, string namedPipeReadPath) + public AnonymousPipesHost(string writHandle, string readHandle) { - outWriter = new StreamWriter(OpenNamedPipe(namedPipeWritePath, FileAccess.Write), UTF8NoBOM); - // Flush the data to the Stream after each write, otherwise the server will wait for input endlessly! + outWriter = new StreamWriter(OpenAnonymousPipe(writHandle, FileAccess.Write), UTF8NoBOM); + // Flush the data to the Stream after each write, otherwise the host process will wait for input endlessly! outWriter.AutoFlush = true; - inReader = new StreamReader(OpenNamedPipe(namedPipeReadPath, FileAccess.Read), UTF8NoBOM, detectEncodingFromByteOrderMarks: false); + inReader = new StreamReader(OpenAnonymousPipe(readHandle, FileAccess.Read), UTF8NoBOM, detectEncodingFromByteOrderMarks: false); } - private Stream OpenNamedPipe(string namedPipePath, FileAccess access) - { -#if NETSTANDARD - if (RuntimeInformation.IsWindows()) - { - return WindowsSyscallCallHelper.OpenNamedPipe(namedPipePath, access); - } -#endif - - FileShare share = RuntimeInformation.IsWindows() ? FileShare.None : FileShare.ReadWrite; - return new FileStream(namedPipePath, FileMode.Open, access, share); - } + private Stream OpenAnonymousPipe(string fileHandle, FileAccess access) + => new FileStream(new SafeFileHandle(new IntPtr(int.Parse(fileHandle)), ownsHandle: true), access); public void Dispose() { @@ -66,55 +53,20 @@ public void SendSignal(HostSignal hostSignal) public void ReportResults(RunResults runResults) => runResults.Print(outWriter); [PublicAPI] // called from generated code - public static bool TryGetNamedPipePath(string[] args, out string namedPipeWritePath, out string namedPipeReadPath) + public static bool TryGetFileHandles(string[] args, out string writeHandle, out string readHandle) { for (int i = 0; i < args.Length; i++) { - if (args[i] == NamedPipeArgument) + if (args[i] == AnonymousPipesDescriptors) { - namedPipeWritePath = args[i + 1]; // IndexOutOfRangeException means a bug (incomplete data) - namedPipeReadPath = args[i + 2]; + writeHandle = args[i + 1]; // IndexOutOfRangeException means a bug (incomplete data) + readHandle = args[i + 2]; return true; } } - namedPipeWritePath = namedPipeReadPath = null; + writeHandle = readHandle = null; return false; } - - // logic based on https://github.com/dotnet/runtime/blob/a54a823ece1094dd05b7380614bd43566834a8f9/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs#L154 - internal static (string serverName, string clientPath) GenerateNamedPipeNames() - { - string guid = Guid.NewGuid().ToString("N"); - if (RuntimeInformation.IsWindows()) - { - return (guid, Path.GetFullPath($@"\\.\pipe\{guid}")); - } - - return ($"/tmp/{guid}", $"/tmp/{guid}"); - } - - internal static Stream CreateServer(string serverName, string filePath, PipeDirection pipeDirection) - { - if (RuntimeInformation.IsWindows()) - { - return new NamedPipeServerStream(serverName, pipeDirection, maxNumberOfServerInstances: 1); - } - else - { - // mkfifo is not supported by WASM, but it's not a problem, as for WASM the host process is always .NET - // and the benchmark process is actual WASM (it just opens the file using FileStream) - int fd = mkfifo(filePath, 438); // 438 stands for 666 in octal - if (fd == -1) - { - throw new Exception("Unable to create named pipe"); - } - - return new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite, bufferSize: 1); - } - - [DllImport("libc", SetLastError = true)] - static extern int mkfifo(string path, int mode); - } } } diff --git a/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs b/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs index 47be806ab4..fd97997aa0 100644 --- a/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs +++ b/src/BenchmarkDotNet/Engines/NoAcknowledgementConsoleHost.cs @@ -4,7 +4,7 @@ namespace BenchmarkDotNet.Engines { - // this class is used only when somebody manually launches benchmarking .exe without providing named pipe path + // this class is used only when somebody manually launches benchmarking .exe without providing anonymous pipes file descriptors public sealed class NoAcknowledgementConsoleHost : IHost { private readonly TextWriter outWriter; diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs index a959dcc4ec..df381618ed 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs @@ -4,7 +4,6 @@ using System.IO.Pipes; using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Engines; -using BenchmarkDotNet.Portability; using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Loggers @@ -13,11 +12,11 @@ internal class SynchronousProcessOutputLoggerWithDiagnoser { private readonly ILogger logger; private readonly IDiagnoser diagnoser; - private readonly Stream inputFromBenchmark, acknowledgments; + private readonly AnonymousPipeServerStream inputFromBenchmark, acknowledgments; private readonly DiagnoserActionParameters diagnoserActionParameters; public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process process, IDiagnoser diagnoser, - BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, Stream inputFromBenchmark, Stream acknowledgments) + BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, AnonymousPipeServerStream inputFromBenchmark, AnonymousPipeServerStream acknowledgments) { this.logger = logger; this.diagnoser = diagnoser; @@ -35,15 +34,8 @@ public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process proce internal void ProcessInput() { - if (RuntimeInformation.IsWindows()) - { - // wait for the benchmark app to connect first! - ((NamedPipeServerStream)inputFromBenchmark).WaitForConnection(); - ((NamedPipeServerStream)acknowledgments).WaitForConnection(); - } - - using StreamReader reader = new (inputFromBenchmark, NamedPipeHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); - using StreamWriter writer = new (acknowledgments, NamedPipeHost.UTF8NoBOM, bufferSize: 1); + using StreamReader reader = new (inputFromBenchmark, AnonymousPipesHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); + using StreamWriter writer = new (acknowledgments, AnonymousPipesHost.UTF8NoBOM, bufferSize: 1); // Flush the data to the Stream after each write, otherwise the client will wait for input endlessly! writer.AutoFlush = true; string line = null; diff --git a/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs b/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs deleted file mode 100644 index 3953002247..0000000000 --- a/src/BenchmarkDotNet/Portability/WindowsSyscallCallHelper.cs +++ /dev/null @@ -1,67 +0,0 @@ -using Microsoft.Win32.SafeHandles; -using System; -using System.IO; -using System.Runtime.InteropServices; - -namespace BenchmarkDotNet.Portability -{ - internal static class WindowsSyscallCallHelper - { - // Before https://github.com/dotnet/runtime/pull/54676 (.NET 6) - // .NET was not allowing to open named pipes using FileStream(path) - // So here we go, calling sys-call directly... - internal static FileStream OpenNamedPipe(string namePipePath, FileAccess fileAccess) - { - // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs#L23-L45 - int access = fileAccess == FileAccess.Read ? unchecked((int)0x80000000) : 0x40000000; // Generic Read & Write - SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(HandleInheritability.None); - - SafeFileHandle sfh = CreateFileW(namePipePath, access, FileShare.None, ref secAttrs, FileMode.Open, 0, hTemplateFile: IntPtr.Zero); - if (sfh.IsInvalid) - { - int lastError = Marshal.GetLastWin32Error(); - throw new Exception($"Unable to open Named Pipe {namePipePath}. Last error: {lastError:X}"); - } - - return new FileStream(sfh, fileAccess); - - [DllImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true)] - static extern SafeFileHandle CreateFileW( - [MarshalAs(UnmanagedType.LPWStr)] string lpFileName, - int dwDesiredAccess, - FileShare dwShareMode, - ref SECURITY_ATTRIBUTES secAttrs, - FileMode dwCreationDisposition, - int dwFlagsAndAttributes, - IntPtr hTemplateFile); - } - - // copied from https://github.com/dotnet/runtime/blob/c59b5171e4fb2b000108bec965f8ce443cb95a12/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Windows.cs#L568 - private static unsafe SECURITY_ATTRIBUTES GetSecAttrs(HandleInheritability inheritability) - { - SECURITY_ATTRIBUTES secAttrs = new SECURITY_ATTRIBUTES - { - nLength = (uint)sizeof(SECURITY_ATTRIBUTES), - bInheritHandle = ((inheritability & HandleInheritability.Inheritable) != 0) ? BOOL.TRUE : BOOL.FALSE - }; - - return secAttrs; - } - - // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Windows/Interop.BOOL.cs - internal enum BOOL : int - { - FALSE = 0, - TRUE = 1, - } - - // copied from https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SECURITY_ATTRIBUTES.cs - [StructLayout(LayoutKind.Sequential)] - internal struct SECURITY_ATTRIBUTES - { - internal uint nLength; - internal IntPtr lpSecurityDescriptor; - internal BOOL bInheritHandle; - } - } -} diff --git a/src/BenchmarkDotNet/Running/BenchmarkId.cs b/src/BenchmarkDotNet/Running/BenchmarkId.cs index 22b27fb055..c223c8d82b 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkId.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkId.cs @@ -16,20 +16,10 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) Value = value; FullBenchmarkName = GetBenchmarkName(benchmarkCase); JobId = benchmarkCase.Job.Id; - (NamedPipeServer_CTS, NamedPipeClient_CTS) = NamedPipeHost.GenerateNamedPipeNames(); - (NamedPipeServer_STC, NamedPipeClient_STC) = NamedPipeHost.GenerateNamedPipeNames(); } public int Value { get; } - internal string NamedPipeServer_CTS { get; } - - internal string NamedPipeClient_CTS { get; } - - internal string NamedPipeServer_STC { get; } - - internal string NamedPipeClient_STC { get; } - private string JobId { get; } private string FullBenchmarkName { get; } @@ -40,7 +30,7 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) public override int GetHashCode() => Value; - public string ToArguments() => $"{NamedPipeHost.NamedPipeArgument} {NamedPipeClient_CTS.Escape()} {NamedPipeClient_STC.Escape()} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; + public string ToArguments(string fromBenchmark, string toBenchmark) => $"{AnonymousPipesHost.AnonymousPipesDescriptors} {fromBenchmark} {toBenchmark} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; public override string ToString() => Value.ToString(); diff --git a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt index 16804411e2..65dc0a24e6 100644 --- a/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt +++ b/src/BenchmarkDotNet/Templates/BenchmarkProgram.txt @@ -25,8 +25,8 @@ namespace BenchmarkDotNet.Autogenerated private static System.Int32 AfterAssemblyLoadingAttached(System.String[] args) { BenchmarkDotNet.Engines.IHost host; // this variable name is used by CodeGenerator.GetCoreRtSwitch, do NOT change it - if (BenchmarkDotNet.Engines.NamedPipeHost.TryGetNamedPipePath(args, out System.String namedPipeWritePath, out System.String namedPipeReadPath)) - host = new BenchmarkDotNet.Engines.NamedPipeHost(namedPipeWritePath, namedPipeReadPath); + if (BenchmarkDotNet.Engines.AnonymousPipesHost.TryGetFileHandles(args, out System.String writeHandle, out System.String readHandle)) + host = new BenchmarkDotNet.Engines.AnonymousPipesHost(writeHandle, readHandle); else host = new BenchmarkDotNet.Engines.NoAcknowledgementConsoleHost(); diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 3a86daa912..249f2c144d 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -63,18 +63,19 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, IResolver resolver, int launchIndex) { + using AnonymousPipeServerStream inputFromBenchmark = new AnonymousPipeServerStream(PipeDirection.In, HandleInheritability.Inheritable); + using AnonymousPipeServerStream acknowledgments = new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.Inheritable); + var startInfo = DotNetCliCommandExecutor.BuildStartInfo( CustomDotNetCliPath, artifactsPaths.BinariesDirectoryPath, - $"{executableName.Escape()} {benchmarkId.ToArguments()}", + $"{executableName.Escape()} {benchmarkId.ToArguments(inputFromBenchmark.GetClientHandleAsString(), acknowledgments.GetClientHandleAsString())}", redirectStandardOutput: false, redirectStandardInput: false, redirectStandardError: false); // #1629 startInfo.SetEnvironmentVariables(benchmarkCase, resolver); - using (Stream inputFromBenchmark = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_CTS, benchmarkId.NamedPipeClient_CTS, PipeDirection.In)) - using (Stream acknowledgments = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_STC, benchmarkId.NamedPipeClient_STC, PipeDirection.Out)) using (var process = new Process { StartInfo = startInfo }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 27b73960ce..4413e453d2 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -25,7 +25,6 @@ public class Executor : IExecutor public ExecuteResult Execute(ExecuteParameters executeParameters) { string exePath = executeParameters.BuildResult.ArtifactsPaths.ExecutablePath; - string args = executeParameters.BenchmarkId.ToArguments(); if (!File.Exists(exePath)) { @@ -33,16 +32,19 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) } return Execute(executeParameters.BenchmarkCase, executeParameters.BenchmarkId, executeParameters.Logger, executeParameters.BuildResult.ArtifactsPaths, - args, executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex); + executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex); } private ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, - string args, IDiagnoser diagnoser, IResolver resolver, int launchIndex) + IDiagnoser diagnoser, IResolver resolver, int launchIndex) { try { - using (Stream inputFromBenchmark = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_CTS, benchmarkId.NamedPipeClient_CTS, PipeDirection.In)) - using (Stream acknowledgments = NamedPipeHost.CreateServer(benchmarkId.NamedPipeServer_STC, benchmarkId.NamedPipeClient_STC, PipeDirection.Out)) + using AnonymousPipeServerStream inputFromBenchmark = new AnonymousPipeServerStream(PipeDirection.In, HandleInheritability.Inheritable); + using AnonymousPipeServerStream acknowledgments = new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.Inheritable); + + string args = benchmarkId.ToArguments(inputFromBenchmark.GetClientHandleAsString(), acknowledgments.GetClientHandleAsString()); + using (var process = new Process { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver) }) using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) { From 30f7337c21270e7b1a12a494d6342409d76ee1cf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 12 Sep 2022 13:32:49 +0200 Subject: [PATCH 07/22] we can't use pipes on WASM --- ...NamedPipeHost.cs => AnonymousPipesHost.cs} | 0 .../Loggers/AsyncProcessOutputReader.cs | 25 ++-- ...hronousProcessOutputLoggerWithDiagnoser.cs | 1 + src/BenchmarkDotNet/Running/BenchmarkId.cs | 4 +- src/BenchmarkDotNet/Toolchains/Executor.cs | 10 -- .../Toolchains/MonoWasm/WasmExecutor.cs | 114 ++++++++++++++++++ .../Toolchains/MonoWasm/WasmToolChain.cs | 2 +- 7 files changed, 136 insertions(+), 20 deletions(-) rename src/BenchmarkDotNet/Engines/{NamedPipeHost.cs => AnonymousPipesHost.cs} (100%) create mode 100644 src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs diff --git a/src/BenchmarkDotNet/Engines/NamedPipeHost.cs b/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs similarity index 100% rename from src/BenchmarkDotNet/Engines/NamedPipeHost.cs rename to src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs diff --git a/src/BenchmarkDotNet/Loggers/AsyncProcessOutputReader.cs b/src/BenchmarkDotNet/Loggers/AsyncProcessOutputReader.cs index 694bcf6352..480b7513cc 100644 --- a/src/BenchmarkDotNet/Loggers/AsyncProcessOutputReader.cs +++ b/src/BenchmarkDotNet/Loggers/AsyncProcessOutputReader.cs @@ -11,16 +11,16 @@ internal class AsyncProcessOutputReader : IDisposable { private readonly Process process; private readonly ConcurrentQueue output, error; + private readonly bool logOutput, readStandardError; + private readonly ILogger logger; private long status; - private bool logOutput; - private ILogger logger; - internal AsyncProcessOutputReader(Process process, bool logOutput = false, ILogger logger = null) + internal AsyncProcessOutputReader(Process process, bool logOutput = false, ILogger logger = null, bool readStandardError = true) { if (!process.StartInfo.RedirectStandardOutput) throw new NotSupportedException("set RedirectStandardOutput to true first"); - if (!process.StartInfo.RedirectStandardError) + if (readStandardError && !process.StartInfo.RedirectStandardError) throw new NotSupportedException("set RedirectStandardError to true first"); if (logOutput && logger == null) throw new ArgumentException($"{nameof(logger)} cannot be null when {nameof(logOutput)} is true"); @@ -31,6 +31,7 @@ internal AsyncProcessOutputReader(Process process, bool logOutput = false, ILogg status = (long)Status.Created; this.logOutput = logOutput; this.logger = logger; + this.readStandardError = readStandardError; } public void Dispose() @@ -48,7 +49,9 @@ internal void BeginRead() Attach(); process.BeginOutputReadLine(); - process.BeginErrorReadLine(); + + if (readStandardError) + process.BeginErrorReadLine(); } internal void CancelRead() @@ -57,7 +60,9 @@ internal void CancelRead() throw new InvalidOperationException("Only a started reader can be stopped"); process.CancelOutputRead(); - process.CancelErrorRead(); + + if (readStandardError) + process.CancelErrorRead(); Detach(); } @@ -83,13 +88,17 @@ internal void StopRead() private void Attach() { process.OutputDataReceived += ProcessOnOutputDataReceived; - process.ErrorDataReceived += ProcessOnErrorDataReceived; + + if (readStandardError) + process.ErrorDataReceived += ProcessOnErrorDataReceived; } private void Detach() { process.OutputDataReceived -= ProcessOnOutputDataReceived; - process.ErrorDataReceived -= ProcessOnErrorDataReceived; + + if (readStandardError) + process.ErrorDataReceived -= ProcessOnErrorDataReceived; } private void ProcessOnOutputDataReceived(object sender, DataReceivedEventArgs e) diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs index df381618ed..008929cc4d 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs @@ -42,6 +42,7 @@ internal void ProcessInput() while ((line = reader.ReadLine()) is not null) { + // TODO: implement Silent mode here logger.WriteLine(LogKind.Default, line); if (!line.StartsWith("//")) diff --git a/src/BenchmarkDotNet/Running/BenchmarkId.cs b/src/BenchmarkDotNet/Running/BenchmarkId.cs index c223c8d82b..acfc89ce85 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkId.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkId.cs @@ -30,7 +30,9 @@ public BenchmarkId(int value, BenchmarkCase benchmarkCase) public override int GetHashCode() => Value; - public string ToArguments(string fromBenchmark, string toBenchmark) => $"{AnonymousPipesHost.AnonymousPipesDescriptors} {fromBenchmark} {toBenchmark} --benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; + public string ToArguments() => $"--benchmarkName {FullBenchmarkName.Escape()} --job {JobId.Escape()} --benchmarkId {Value}"; + + public string ToArguments(string fromBenchmark, string toBenchmark) => $"{AnonymousPipesHost.AnonymousPipesDescriptors} {fromBenchmark} {toBenchmark} {ToArguments()}"; public override string ToString() => Value.ToString(); diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 4413e453d2..5b13615825 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -111,7 +111,6 @@ private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsP string exePath = artifactsPaths.ExecutablePath; var runtime = benchmarkCase.GetRuntime(); - // TODO: use resolver switch (runtime) { @@ -125,15 +124,6 @@ private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsP start.FileName = mono.CustomPath ?? "mono"; start.Arguments = GetMonoArguments(benchmarkCase.Job, exePath, args, resolver); break; - case WasmRuntime wasm: - start.FileName = wasm.JavaScriptEngine; - start.RedirectStandardInput = false; - - string main_js = runtime.RuntimeMoniker < RuntimeMoniker.WasmNet70 ? "main.js" : "test-main.js"; - - start.Arguments = $"{wasm.JavaScriptEngineArguments} {main_js} -- --run {artifactsPaths.ProgramName}.dll {args} "; - start.WorkingDirectory = artifactsPaths.BinariesDirectoryPath; - break; case MonoAotLLVMRuntime _: start.FileName = exePath; start.Arguments = args; diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs new file mode 100644 index 0000000000..2928637e5f --- /dev/null +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs @@ -0,0 +1,114 @@ +using BenchmarkDotNet.Characteristics; +using BenchmarkDotNet.Diagnosers; +using BenchmarkDotNet.Engines; +using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Extensions; +using BenchmarkDotNet.Helpers; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.Parameters; +using BenchmarkDotNet.Toolchains.Results; +using System; +using System.Collections.Immutable; +using System.Diagnostics; +using System.IO; +using System.Linq; + +namespace BenchmarkDotNet.Toolchains.MonoWasm +{ + internal class WasmExecutor : IExecutor + { + public ExecuteResult Execute(ExecuteParameters executeParameters) + { + string exePath = executeParameters.BuildResult.ArtifactsPaths.ExecutablePath; + + if (!File.Exists(exePath)) + { + return ExecuteResult.CreateFailed(); + } + + return Execute(executeParameters.BenchmarkCase, executeParameters.BenchmarkId, executeParameters.Logger, executeParameters.BuildResult.ArtifactsPaths, + executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex); + } + + private static ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, + IDiagnoser diagnoser, IResolver resolver, int launchIndex) + { + try + { + using (Process process = CreateProcess(benchmarkCase, artifactsPaths, benchmarkId.ToArguments(), resolver)) + using (ConsoleExitHandler consoleExitHandler = new (process, logger)) + using (AsyncProcessOutputReader processOutputReader = new (process, readStandardError: false)) + { + diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); + + return Execute(process, benchmarkCase, processOutputReader, logger, consoleExitHandler, launchIndex); + } + } + finally + { + diagnoser?.Handle(HostSignal.AfterProcessExit, new DiagnoserActionParameters(null, benchmarkCase, benchmarkId)); + } + } + + private static Process CreateProcess(BenchmarkCase benchmarkCase, ArtifactsPaths artifactsPaths, string args, IResolver resolver) + { + WasmRuntime runtime = (WasmRuntime)benchmarkCase.GetRuntime(); + string mainJs = runtime.RuntimeMoniker < RuntimeMoniker.WasmNet70 ? "main.js" : "test-main.js"; + + var start = new ProcessStartInfo + { + FileName = runtime.JavaScriptEngine, + Arguments = $"{runtime.JavaScriptEngineArguments} {mainJs} -- --run {artifactsPaths.ProgramName}.dll {args} ", + WorkingDirectory = artifactsPaths.BinariesDirectoryPath, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardInput = false, // not supported by WASM! + RedirectStandardError = false, // #1629 + CreateNoWindow = true + }; + + start.SetEnvironmentVariables(benchmarkCase, resolver); + + return new Process() { StartInfo = start }; + } + + private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCase, AsyncProcessOutputReader processOutputReader, + ILogger logger, ConsoleExitHandler consoleExitHandler, int launchIndex) + { + logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); + + process.Start(); + + processOutputReader.BeginRead(); + + process.EnsureHighPriority(logger); + if (benchmarkCase.Job.Environment.HasValue(EnvironmentMode.AffinityCharacteristic)) + { + process.TrySetAffinity(benchmarkCase.Job.Environment.Affinity, logger); + } + + if (!process.WaitForExit(milliseconds: (int)TimeSpan.FromMinutes(10).TotalMilliseconds)) + { + logger.WriteLineInfo("// The benchmarking process did not finish within 10 minutes, it's going to get force killed now."); + + processOutputReader.StopRead(); + consoleExitHandler.KillProcessTree(); + } + else + { + processOutputReader.StopRead(); + } + + ImmutableArray outputLines = processOutputReader.GetOutputLines(); + + return new ExecuteResult(true, + process.HasExited ? process.ExitCode : null, + process.Id, + outputLines.Where(line => !line.StartsWith("//")).ToArray(), + outputLines.Where(line => line.StartsWith("//")).ToArray(), + launchIndex); + } + } +} diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolChain.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolChain.cs index 3c7e75adac..326957d74a 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolChain.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolChain.cs @@ -45,7 +45,7 @@ public static IToolchain From(NetCoreAppSettings netCoreAppSettings) // aot builds can be very slow logOutput: netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm, retryFailedBuildWithNoDeps: false), - new Executor(), + new WasmExecutor(), netCoreAppSettings.CustomDotNetCliPath); } } From 1883b520350bae5d37021480c7c968d06600582c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 12 Sep 2022 14:07:47 +0200 Subject: [PATCH 08/22] ensure benchmark process output is redirected, read the output in non-blocking way and expose it to the end-users so they can do whatever they want with it --- src/BenchmarkDotNet/Engines/GcStats.cs | 2 +- src/BenchmarkDotNet/Engines/ThreadingStats.cs | 2 +- ...OutputLoggerWithDiagnoser.cs => Broker.cs} | 18 +++---- .../Reports/BenchmarkReportExtensions.cs | 2 +- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 23 ++++++--- src/BenchmarkDotNet/Toolchains/Executor.cs | 41 +++++++++------- .../Toolchains/MonoWasm/WasmExecutor.cs | 1 + .../Toolchains/Results/ExecuteResult.cs | 48 +++++++++++-------- .../BuildTimeoutTests.cs | 3 +- .../CustomEngineTests.cs | 2 +- .../ToolchainTest.cs | 2 +- .../Mocks/MockFactory.cs | 2 +- 12 files changed, 87 insertions(+), 59 deletions(-) rename src/BenchmarkDotNet/Loggers/{SynchronousProcessOutputLoggerWithDiagnoser.cs => Broker.cs} (81%) diff --git a/src/BenchmarkDotNet/Engines/GcStats.cs b/src/BenchmarkDotNet/Engines/GcStats.cs index 2f2552b4d0..1d3812173e 100644 --- a/src/BenchmarkDotNet/Engines/GcStats.cs +++ b/src/BenchmarkDotNet/Engines/GcStats.cs @@ -9,7 +9,7 @@ namespace BenchmarkDotNet.Engines { public struct GcStats : IEquatable { - internal const string ResultsLinePrefix = "GC: "; + internal const string ResultsLinePrefix = "// GC: "; public static readonly long AllocationQuantum = CalculateAllocationQuantumSize(); diff --git a/src/BenchmarkDotNet/Engines/ThreadingStats.cs b/src/BenchmarkDotNet/Engines/ThreadingStats.cs index d196577b75..6afa5fec11 100644 --- a/src/BenchmarkDotNet/Engines/ThreadingStats.cs +++ b/src/BenchmarkDotNet/Engines/ThreadingStats.cs @@ -5,7 +5,7 @@ namespace BenchmarkDotNet.Engines { public struct ThreadingStats : IEquatable { - internal const string ResultsLinePrefix = "Threading: "; + internal const string ResultsLinePrefix = "// Threading: "; // BDN targets .NET Standard 2.0, these properties are not part of .NET Standard 2.0, were added in .NET Core 3.0 private static readonly Func GetCompletedWorkItemCountDelegate = CreateGetterDelegate(typeof(ThreadPool), nameof(CompletedWorkItemCount)); diff --git a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs b/src/BenchmarkDotNet/Loggers/Broker.cs similarity index 81% rename from src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs rename to src/BenchmarkDotNet/Loggers/Broker.cs index 008929cc4d..e915d61eca 100644 --- a/src/BenchmarkDotNet/Loggers/SynchronousProcessOutputLoggerWithDiagnoser.cs +++ b/src/BenchmarkDotNet/Loggers/Broker.cs @@ -8,14 +8,14 @@ namespace BenchmarkDotNet.Loggers { - internal class SynchronousProcessOutputLoggerWithDiagnoser + internal class Broker { private readonly ILogger logger; private readonly IDiagnoser diagnoser; private readonly AnonymousPipeServerStream inputFromBenchmark, acknowledgments; private readonly DiagnoserActionParameters diagnoserActionParameters; - public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process process, IDiagnoser diagnoser, + public Broker(ILogger logger, Process process, IDiagnoser diagnoser, BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, AnonymousPipeServerStream inputFromBenchmark, AnonymousPipeServerStream acknowledgments) { this.logger = logger; @@ -24,15 +24,15 @@ public SynchronousProcessOutputLoggerWithDiagnoser(ILogger logger, Process proce this.acknowledgments = acknowledgments; diagnoserActionParameters = new DiagnoserActionParameters(process, benchmarkCase, benchmarkId); - LinesWithResults = new List(); - LinesWithExtraOutput = new List(); + Results = new List(); + PrefixedOutput = new List(); } - internal List LinesWithResults { get; } + internal List Results { get; } - internal List LinesWithExtraOutput { get; } + internal List PrefixedOutput { get; } - internal void ProcessInput() + internal void ProcessData() { using StreamReader reader = new (inputFromBenchmark, AnonymousPipesHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false); using StreamWriter writer = new (acknowledgments, AnonymousPipesHost.UTF8NoBOM, bufferSize: 1); @@ -47,7 +47,7 @@ internal void ProcessInput() if (!line.StartsWith("//")) { - LinesWithResults.Add(line); + Results.Add(line); } else if (Engine.Signals.TryGetSignal(line, out var signal)) { @@ -64,7 +64,7 @@ internal void ProcessInput() } else if (!string.IsNullOrEmpty(line)) { - LinesWithExtraOutput.Add(line); + PrefixedOutput.Add(line); } } } diff --git a/src/BenchmarkDotNet/Reports/BenchmarkReportExtensions.cs b/src/BenchmarkDotNet/Reports/BenchmarkReportExtensions.cs index 6681ccb210..4f163dc66e 100644 --- a/src/BenchmarkDotNet/Reports/BenchmarkReportExtensions.cs +++ b/src/BenchmarkDotNet/Reports/BenchmarkReportExtensions.cs @@ -24,7 +24,7 @@ private static string GetInfoFromOutput(this BenchmarkReport report, string pref { return ( from executeResults in report.ExecuteResults - from extraOutputLine in executeResults.ExtraOutput.Where(line => line.StartsWith(prefix)) + from extraOutputLine in executeResults.PrefixedLines.Where(line => line.StartsWith(prefix)) select extraOutputLine.Substring(prefix.Length)).FirstOrDefault(); } } diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 249f2c144d..260b782345 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -70,16 +70,17 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, CustomDotNetCliPath, artifactsPaths.BinariesDirectoryPath, $"{executableName.Escape()} {benchmarkId.ToArguments(inputFromBenchmark.GetClientHandleAsString(), acknowledgments.GetClientHandleAsString())}", - redirectStandardOutput: false, + redirectStandardOutput: true, redirectStandardInput: false, redirectStandardError: false); // #1629 startInfo.SetEnvironmentVariables(benchmarkCase, resolver); - using (var process = new Process { StartInfo = startInfo }) - using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) + using (Process process = new () { StartInfo = startInfo }) + using (ConsoleExitHandler consoleExitHandler = new (process, logger)) + using (AsyncProcessOutputReader processOutputReader = new (process, logOutput: true, logger, readStandardError: false)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); + var loggerWithDiagnoser = new Broker(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); @@ -93,20 +94,28 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, process.TrySetAffinity(benchmarkCase.Job.Environment.Affinity, logger); } - loggerWithDiagnoser.ProcessInput(); + processOutputReader.BeginRead(); + + loggerWithDiagnoser.ProcessData(); if (!process.WaitForExit(milliseconds: (int)ExecuteParameters.ProcessExitTimeout.TotalMilliseconds)) { logger.WriteLineInfo($"// The benchmarking process did not quit within {ExecuteParameters.ProcessExitTimeout.TotalSeconds} seconds, it's going to get force killed now."); + processOutputReader.StopRead(); consoleExitHandler.KillProcessTree(); } + else + { + processOutputReader.StopRead(); + } return new ExecuteResult(true, process.HasExited ? process.ExitCode : null, process.Id, - loggerWithDiagnoser.LinesWithResults, - loggerWithDiagnoser.LinesWithExtraOutput, + loggerWithDiagnoser.Results, + loggerWithDiagnoser.PrefixedOutput, + processOutputReader.GetOutputLines(), launchIndex); } } diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 5b13615825..44352e477b 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -35,24 +35,25 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) executeParameters.Diagnoser, executeParameters.Resolver, executeParameters.LaunchIndex); } - private ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, + private static ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, ArtifactsPaths artifactsPaths, IDiagnoser diagnoser, IResolver resolver, int launchIndex) { try { - using AnonymousPipeServerStream inputFromBenchmark = new AnonymousPipeServerStream(PipeDirection.In, HandleInheritability.Inheritable); - using AnonymousPipeServerStream acknowledgments = new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.Inheritable); + using AnonymousPipeServerStream inputFromBenchmark = new (PipeDirection.In, HandleInheritability.Inheritable); + using AnonymousPipeServerStream acknowledgments = new (PipeDirection.Out, HandleInheritability.Inheritable); string args = benchmarkId.ToArguments(inputFromBenchmark.GetClientHandleAsString(), acknowledgments.GetClientHandleAsString()); - using (var process = new Process { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver) }) - using (var consoleExitHandler = new ConsoleExitHandler(process, logger)) + using (Process process = new () { StartInfo = CreateStartInfo(benchmarkCase, artifactsPaths, args, resolver) }) + using (ConsoleExitHandler consoleExitHandler = new (process, logger)) + using (AsyncProcessOutputReader processOutputReader = new (process, logOutput: true, logger, readStandardError: false)) { - var loggerWithDiagnoser = new SynchronousProcessOutputLoggerWithDiagnoser(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); + Broker broker = new (logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); - return Execute(process, benchmarkCase, loggerWithDiagnoser, logger, consoleExitHandler, launchIndex); + return Execute(process, benchmarkCase, broker, logger, consoleExitHandler, launchIndex, processOutputReader); } } finally @@ -61,8 +62,8 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId benchmark } } - private ExecuteResult Execute(Process process, BenchmarkCase benchmarkCase, SynchronousProcessOutputLoggerWithDiagnoser loggerWithDiagnoser, - ILogger logger, ConsoleExitHandler consoleExitHandler, int launchIndex) + private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCase, Broker broker, + ILogger logger, ConsoleExitHandler consoleExitHandler, int launchIndex, AsyncProcessOutputReader processOutputReader) { logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); @@ -74,32 +75,40 @@ private ExecuteResult Execute(Process process, BenchmarkCase benchmarkCase, Sync process.TrySetAffinity(benchmarkCase.Job.Environment.Affinity, logger); } - loggerWithDiagnoser.ProcessInput(); + processOutputReader.BeginRead(); + + broker.ProcessData(); if (!process.WaitForExit(milliseconds: (int)ExecuteParameters.ProcessExitTimeout.TotalMilliseconds)) { logger.WriteLineInfo("// The benchmarking process did not quit on time, it's going to get force killed now."); + processOutputReader.StopRead(); consoleExitHandler.KillProcessTree(); } + else + { + processOutputReader.StopRead(); + } - if (loggerWithDiagnoser.LinesWithResults.Any(line => line.Contains("BadImageFormatException"))) + if (broker.Results.Any(line => line.Contains("BadImageFormatException"))) logger.WriteLineError("You are probably missing AnyCPU in your .csproj file."); return new ExecuteResult(true, process.HasExited ? process.ExitCode : null, process.Id, - loggerWithDiagnoser.LinesWithResults, - loggerWithDiagnoser.LinesWithExtraOutput, + broker.Results, + broker.PrefixedOutput, + processOutputReader.GetOutputLines(), launchIndex); } - private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsPaths artifactsPaths, string args, IResolver resolver) + private static ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsPaths artifactsPaths, string args, IResolver resolver) { var start = new ProcessStartInfo { UseShellExecute = false, - RedirectStandardOutput = false, + RedirectStandardOutput = true, RedirectStandardInput = false, RedirectStandardError = false, // #1629 CreateNoWindow = true, @@ -135,7 +144,7 @@ private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsP return start; } - private string GetMonoArguments(Job job, string exePath, string args, IResolver resolver) + private static string GetMonoArguments(Job job, string exePath, string args, IResolver resolver) { var arguments = job.HasValue(InfrastructureMode.ArgumentsCharacteristic) ? job.ResolveValue(InfrastructureMode.ArgumentsCharacteristic, resolver).OfType().ToArray() diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs index 2928637e5f..74e26381f6 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs @@ -108,6 +108,7 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas process.Id, outputLines.Where(line => !line.StartsWith("//")).ToArray(), outputLines.Where(line => line.StartsWith("//")).ToArray(), + outputLines, launchIndex); } } diff --git a/src/BenchmarkDotNet/Toolchains/Results/ExecuteResult.cs b/src/BenchmarkDotNet/Toolchains/Results/ExecuteResult.cs index 51a4614541..805f05677f 100644 --- a/src/BenchmarkDotNet/Toolchains/Results/ExecuteResult.cs +++ b/src/BenchmarkDotNet/Toolchains/Results/ExecuteResult.cs @@ -15,10 +15,17 @@ public class ExecuteResult public int? ProcessId { get; } public IReadOnlyList Errors => errors; public IReadOnlyList Measurements => measurements; - public IReadOnlyList ExtraOutput { get; } + /// + /// All lines printed to standard output by the Benchmark process + /// + public IReadOnlyList StandardOutput { get; } + /// + /// Lines reported by the Benchmark process that are starting with "//" + /// + internal IReadOnlyList PrefixedLines { get; } internal readonly GcStats GcStats; internal readonly ThreadingStats ThreadingStats; - private readonly IReadOnlyList data; + private readonly IReadOnlyList results; private readonly List errors; private readonly List measurements; @@ -26,15 +33,15 @@ public class ExecuteResult // that is why we search for Workload Results as they are produced at the end public bool IsSuccess => Measurements.Any(m => m.Is(IterationMode.Workload, IterationStage.Result)); - public ExecuteResult(bool foundExecutable, int? exitCode, int? processId, IReadOnlyList data, IReadOnlyList linesWithExtraOutput, int launchIndex) + public ExecuteResult(bool foundExecutable, int? exitCode, int? processId, IReadOnlyList results, IReadOnlyList prefixedLines, IReadOnlyList standardOutput, int launchIndex) { FoundExecutable = foundExecutable; - this.data = data; + this.results = results; ProcessId = processId; ExitCode = exitCode; - ExtraOutput = linesWithExtraOutput; - - Parse(data, launchIndex, out measurements, out errors, out GcStats, out ThreadingStats); + PrefixedLines = prefixedLines; + StandardOutput = standardOutput; + Parse(results, prefixedLines, launchIndex, out measurements, out errors, out GcStats, out ThreadingStats); } internal ExecuteResult(List measurements, GcStats gcStats, ThreadingStats threadingStats) @@ -42,7 +49,7 @@ internal ExecuteResult(List measurements, GcStats gcStats, Threadin FoundExecutable = true; ExitCode = 0; errors = new List(); - ExtraOutput = Array.Empty(); + PrefixedLines = Array.Empty(); this.measurements = measurements; GcStats = gcStats; ThreadingStats = threadingStats; @@ -54,7 +61,7 @@ internal static ExecuteResult FromRunResults(RunResults runResults, int exitCode : new ExecuteResult(runResults.GetMeasurements().ToList(), runResults.GCStats, runResults.ThreadingStats); internal static ExecuteResult CreateFailed(int exitCode = -1) - => new ExecuteResult(false, exitCode, default, Array.Empty(), Array.Empty(), 0); + => new ExecuteResult(false, exitCode, default, Array.Empty(), Array.Empty(), Array.Empty(), 0); public override string ToString() => "ExecuteResult: " + (FoundExecutable ? "Found executable" : "Executable not found"); @@ -67,7 +74,7 @@ public void LogIssues(ILogger logger, BuildResult buildResult) // exit code can be different than 0 if the process has hanged at the end // so we check if some results were reported, if not then it was a failure - if (ExitCode != 0 && data.Count == 0) + if (ExitCode != 0 && results.Count == 0) { logger.WriteLineError("ExitCode != 0 and no results reported"); } @@ -83,7 +90,7 @@ public void LogIssues(ILogger logger, BuildResult buildResult) } } - private static void Parse(IReadOnlyList data, int launchIndex, out List measurements, + private static void Parse(IReadOnlyList results, IReadOnlyList prefixedLines, int launchIndex, out List measurements, out List errors, out GcStats gcStats, out ThreadingStats threadingStats) { measurements = new List(); @@ -91,7 +98,16 @@ private static void Parse(IReadOnlyList data, int launchIndex, out List< gcStats = default; threadingStats = default; - foreach (string line in data.Where(text => !string.IsNullOrEmpty(text))) + foreach (string line in results.Where(text => !string.IsNullOrEmpty(text))) + { + Measurement measurement = Measurement.Parse(line, launchIndex); + if (measurement.IterationMode != IterationMode.Unknown) + { + measurements.Add(measurement); + } + } + + foreach (string line in prefixedLines.Where(text => !string.IsNullOrEmpty(text))) { if (line.StartsWith(ValidationErrorReporter.ConsoleErrorPrefix)) { @@ -105,14 +121,6 @@ private static void Parse(IReadOnlyList data, int launchIndex, out List< { threadingStats = ThreadingStats.Parse(line); } - else - { - Measurement measurement = Measurement.Parse(line, launchIndex); - if (measurement.IterationMode != IterationMode.Unknown) - { - measurements.Add(measurement); - } - } } if (errors.Count > 0) diff --git a/tests/BenchmarkDotNet.IntegrationTests/BuildTimeoutTests.cs b/tests/BenchmarkDotNet.IntegrationTests/BuildTimeoutTests.cs index c1b4c75837..5efd67a67e 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/BuildTimeoutTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/BuildTimeoutTests.cs @@ -31,7 +31,8 @@ public void WhenBuildTakesMoreTimeThanTheTimeoutTheBuildIsCancelled() .WithToolchain(NativeAotToolchain.CreateBuilder() .UseNuGet( "6.0.0-rc.1.21420.1", // we test against specific version to keep this test stable - "https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-experimental/nuget/v3/index.json") // using old feed that supports net5.0 + "https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-experimental/nuget/v3/index.json") // using old feed that supports net6.0 + .TargetFrameworkMoniker("net6.0") .ToToolchain())); var summary = CanExecute(config, fullValidation: false); diff --git a/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs b/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs index b1e268673c..5bb9c29bf2 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs @@ -38,7 +38,7 @@ public void CustomEnginesAreSupported() private static void AssertMessageGotDisplayed(Summary summary, string message) { - Assert.True(summary.Reports.Any(report => report.ExecuteResults.Any(executeResult => executeResult.ExtraOutput.Any(line => line == message))), $"{message} should have been printed by custom Engine"); + Assert.True(summary.Reports.Any(report => report.ExecuteResults.Any(executeResult => executeResult.StandardOutput.Any(line => line == message))), $"{message} should have been printed by custom Engine"); } public class SimpleBenchmark diff --git a/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs index 38cac6dfb5..f8cb9686f3 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ToolchainTest.cs @@ -49,7 +49,7 @@ public ExecuteResult Execute(ExecuteParameters executeParameters) { executeParameters.Logger.WriteLine("Executing"); Done = true; - return new ExecuteResult(true, 0, default, Array.Empty(), Array.Empty(), executeParameters.LaunchIndex); + return new ExecuteResult(true, 0, default, Array.Empty(), Array.Empty(), Array.Empty(), executeParameters.LaunchIndex); } } diff --git a/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs b/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs index 27216d830f..813d25f4d1 100644 --- a/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs +++ b/tests/BenchmarkDotNet.Tests/Mocks/MockFactory.cs @@ -71,7 +71,7 @@ private static BenchmarkReport CreateReport(BenchmarkCase benchmarkCase, int n, var measurements = Enumerable.Range(0, n) .Select(index => new Measurement(1, IterationMode.Workload, IterationStage.Result, index + 1, 1, nanoseconds + index).ToString()) .ToList(); - var executeResult = new ExecuteResult(true, 0, default, measurements, new[] { $"// Runtime=extra output line" }, 1); + var executeResult = new ExecuteResult(true, 0, default, measurements, new[] { $"// Runtime=extra output line" }, Array.Empty(), 1); return new BenchmarkReport(true, benchmarkCase, buildResult, buildResult, new List { executeResult }, Array.Empty()); } From 862525b047bb68b6ee99d3b49d2c07d8eaf47454 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 12 Sep 2022 15:24:21 +0200 Subject: [PATCH 09/22] WasmExecutor needs to log output as well --- src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs index 74e26381f6..41417f0675 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs @@ -39,7 +39,7 @@ private static ExecuteResult Execute(BenchmarkCase benchmarkCase, BenchmarkId be { using (Process process = CreateProcess(benchmarkCase, artifactsPaths, benchmarkId.ToArguments(), resolver)) using (ConsoleExitHandler consoleExitHandler = new (process, logger)) - using (AsyncProcessOutputReader processOutputReader = new (process, readStandardError: false)) + using (AsyncProcessOutputReader processOutputReader = new (process, logOutput: true, logger, readStandardError: false)) { diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); From 55156a8e6cf1071eeb66477660026ee29d00999c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 07:42:25 +0200 Subject: [PATCH 10/22] the tests should be using .StandardOutput property now --- .../AllSetupAndCleanupTest.cs | 28 +++++++++------- .../BenchmarkTestExecutor.cs | 8 +++++ .../CustomEngineTests.cs | 12 +++---- .../DryRunTests.cs | 8 ++--- .../GenericBenchmarkTest.cs | 8 ++--- .../GlobalCleanupAttributeTargetTest.cs | 32 ++++++++----------- .../GlobalCleanupAttributeTest.cs | 14 ++++---- .../GlobalSetupAttributeTargetTest.cs | 30 ++++++++--------- .../GlobalSetupAttributeTest.cs | 14 ++++---- .../InnerClassTest.cs | 20 ++++++++---- .../ParamsAttributeStaticTest.cs | 7 ++-- .../ParamsTests.cs | 17 ++++++---- .../RunStrategyTests.cs | 8 ++--- .../SetupAndCleanupTests.cs | 7 ++-- 14 files changed, 116 insertions(+), 97 deletions(-) diff --git a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs index dc4cf9bfee..5bb5e225a2 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs @@ -4,6 +4,7 @@ using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Engines; using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Reports; using BenchmarkDotNet.Tests.Loggers; using BenchmarkDotNet.Tests.XUnit; using Xunit; @@ -45,6 +46,9 @@ public class AllSetupAndCleanupTest : BenchmarkTestExecutor public AllSetupAndCleanupTest(ITestOutputHelper output) : base(output) { } + private static string[] GetActualLogLines(Summary summary) + => GetSingleStandardOutput(summary).Where(line => line.StartsWith(Prefix)).ToArray(); + [Fact] public void AllSetupAndCleanupMethodRunsTest() { @@ -52,9 +56,9 @@ public void AllSetupAndCleanupMethodRunsTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); @@ -88,9 +92,9 @@ public void AllSetupAndCleanupMethodRunsAsyncTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); @@ -124,9 +128,9 @@ public void AllSetupAndCleanupMethodRunsAsyncTaskSetupTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); @@ -160,9 +164,9 @@ public void AllSetupAndCleanupMethodRunsAsyncGenericTaskSetupTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); @@ -206,9 +210,9 @@ public void AllSetupAndCleanupMethodRunsAsyncValueTaskSetupTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); @@ -242,9 +246,9 @@ public void AllSetupAndCleanupMethodRunsAsyncGenericValueTaskSetupTest() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); - var actualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(Prefix)).ToArray(); + var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) Output.WriteLine(line); Assert.Equal(expectedLogLines, actualLogLines); diff --git a/tests/BenchmarkDotNet.IntegrationTests/BenchmarkTestExecutor.cs b/tests/BenchmarkDotNet.IntegrationTests/BenchmarkTestExecutor.cs index a5e3060315..64d270b3d6 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/BenchmarkTestExecutor.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/BenchmarkTestExecutor.cs @@ -8,6 +8,8 @@ using BenchmarkDotNet.Tests.Loggers; using Xunit; using Xunit.Abstractions; +using System.Collections.Generic; +using BenchmarkDotNet.Reports; namespace BenchmarkDotNet.IntegrationTests { @@ -91,5 +93,11 @@ protected IConfig CreateSimpleConfig(OutputLogger logger = null, Job job = null) .AddColumnProvider(DefaultColumnProviders.Instance) .AddAnalyser(DefaultConfig.Instance.GetAnalysers().ToArray()); } + + protected static IReadOnlyList GetSingleStandardOutput(Summary summary) + => summary.Reports.Single().ExecuteResults.Single().StandardOutput; + + protected static IReadOnlyList GetCombinedStandardOutput(Summary summary) + => summary.Reports.SelectMany(r => r.ExecuteResults).SelectMany(e => e.StandardOutput).ToArray(); } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs b/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs index 5bb9c29bf2..29707c917f 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using System.Collections.Generic; using BenchmarkDotNet.Attributes; using Xunit; @@ -31,14 +30,11 @@ public void CustomEnginesAreSupported() var summary = CanExecute(config, fullValidation: false); - AssertMessageGotDisplayed(summary, GlobalSetupMessage); - AssertMessageGotDisplayed(summary, EngineRunMessage); - AssertMessageGotDisplayed(summary, GlobalCleanupMessage); - } + IReadOnlyList standardOutput = GetSingleStandardOutput(summary); - private static void AssertMessageGotDisplayed(Summary summary, string message) - { - Assert.True(summary.Reports.Any(report => report.ExecuteResults.Any(executeResult => executeResult.StandardOutput.Any(line => line == message))), $"{message} should have been printed by custom Engine"); + Assert.Contains(GlobalSetupMessage, standardOutput); + Assert.Contains(EngineRunMessage, standardOutput); + Assert.Contains(GlobalCleanupMessage, standardOutput); } public class SimpleBenchmark diff --git a/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs b/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs index 5a6b6eb76e..31a3e43bd8 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs @@ -32,11 +32,11 @@ public void ColdStart() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); + var standardOutput = GetSingleStandardOutput(summary); - string log = logger.GetLog(); - Assert.Contains($"{CounterPrefix}1", log); - Assert.DoesNotContain($"{CounterPrefix}2", log); + Assert.Contains($"{CounterPrefix}1", standardOutput); + Assert.DoesNotContain($"{CounterPrefix}2", standardOutput); } [DryJob] diff --git a/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs index d5553bd5d6..373968ca03 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs @@ -19,14 +19,14 @@ public void GenericClassesAreSupported() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); var expected1 = $"// ### Benchmark: SerializationLibrary1, Type: {typeof(FlatClassBenchmark).Name} ###"; - Assert.Contains(expected1, logger.GetLog()); + Assert.Contains(expected1, GetCombinedStandardOutput(summary)); logger.ClearLog(); - CanExecute(config); + summary = CanExecute(config); var expected2 = $"// ### Benchmark: SerializationLibrary2, Type: {typeof(DoubleArrayBenchmark).Name} ###"; - Assert.Contains(expected2, logger.GetLog()); + Assert.Contains(expected2, GetCombinedStandardOutput(summary)); } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs index e3060151e1..4e3129dc8a 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Tests.Loggers; using Xunit; @@ -23,26 +24,21 @@ public void GlobalCleanupTargetSpecificMethodTest() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); - string log = logger.GetLog(); + IReadOnlyList standardOuput = GetCombinedStandardOutput(summary); - Assert.Contains(BaselineBenchmarkCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(BaselineBenchmarkCalled + Environment.NewLine) < - log.IndexOf(BaselineGlobalCleanupCalled + Environment.NewLine)); - - Assert.Contains(FirstGlobalCleanupCalled + Environment.NewLine, log); - Assert.Contains(FirstBenchmarkCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(FirstBenchmarkCalled + Environment.NewLine) < - log.IndexOf(FirstGlobalCleanupCalled + Environment.NewLine)); - - Assert.Contains(SecondGlobalCleanupCalled + Environment.NewLine, log); - Assert.Contains(SecondBenchmarkCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(SecondBenchmarkCalled + Environment.NewLine) < - log.IndexOf(SecondGlobalCleanupCalled + Environment.NewLine)); + string[] expected = new string[] + { + BaselineBenchmarkCalled, + BaselineGlobalCleanupCalled, + FirstBenchmarkCalled, + FirstGlobalCleanupCalled, + SecondBenchmarkCalled, + SecondGlobalCleanupCalled + }; + + Assert.Equal(expected, standardOuput); } public class GlobalCleanupAttributeTargetBenchmarks diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs index ee303f5120..fea28b46be 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs @@ -19,13 +19,15 @@ public void GlobalCleanupMethodRunsTest() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); - string log = logger.GetLog(); - Assert.Contains(BenchmarkCalled + System.Environment.NewLine, log); - Assert.True( - log.IndexOf(BenchmarkCalled + System.Environment.NewLine) < - log.IndexOf(GlobalCleanupCalled + System.Environment.NewLine)); + string[] expected = new string[] + { + BenchmarkCalled, + GlobalCleanupCalled + }; + + Assert.Equal(expected, GetSingleStandardOutput(summary)); } public class GlobalCleanupAttributeBenchmarks diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs index 20431f53b9..0e5020c7d8 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Tests.Loggers; using Xunit; @@ -23,24 +24,21 @@ public void GlobalSetupTargetSpecificMethodTest() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); - string log = logger.GetLog(); + IReadOnlyList standardOuput = GetCombinedStandardOutput(summary); - Assert.Contains(BaselineGlobalSetupCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(BaselineGlobalSetupCalled + Environment.NewLine) < - log.IndexOf(BaselineBenchmarkCalled + Environment.NewLine)); - - Assert.Contains(FirstGlobalSetupCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(FirstGlobalSetupCalled + Environment.NewLine) < - log.IndexOf(FirstBenchmarkCalled + Environment.NewLine)); - - Assert.Contains(SecondGlobalSetupCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(SecondGlobalSetupCalled + Environment.NewLine) < - log.IndexOf(SecondBenchmarkCalled + Environment.NewLine)); + string[] expected = new string[] + { + BaselineGlobalSetupCalled, + BaselineBenchmarkCalled, + FirstGlobalSetupCalled, + FirstBenchmarkCalled, + SecondGlobalSetupCalled, + SecondBenchmarkCalled, + }; + + Assert.Equal(expected, standardOuput); } public class GlobalSetupAttributeTargetBenchmarks diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs index 7f1cf0e76b..c1d6ec6da2 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs @@ -19,13 +19,15 @@ public void GlobalSetupMethodRunsTest() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); - string log = logger.GetLog(); - Assert.Contains(GlobalSetupCalled + Environment.NewLine, log); - Assert.True( - log.IndexOf(GlobalSetupCalled + Environment.NewLine) < - log.IndexOf(BenchmarkCalled + Environment.NewLine)); + string[] expected = new string[] + { + GlobalSetupCalled, + BenchmarkCalled + }; + + Assert.Equal(expected, GetSingleStandardOutput(summary)); } public class GlobalSetupAttributeBenchmarks diff --git a/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs b/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs index 08b4fa950b..bc67717a53 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs @@ -10,6 +10,9 @@ namespace BenchmarkDotNet.IntegrationTests // https://github.com/dotnet/BenchmarkDotNet/issues/59 is also related public class InnerClassTest : BenchmarkTestExecutor { + private const string FirstExpectedMessage = "// ### BenchmarkInnerClass method called ###"; + private const string SecondExpectedMessage = "// ### BenchmarkGenericInnerClass method called ###"; + public InnerClassTest(ITestOutputHelper output) : base(output) { } [Fact] @@ -18,12 +21,15 @@ public void InnerClassesAreSupported() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); + + string[] expected = new string[] + { + FirstExpectedMessage, + SecondExpectedMessage + }; - var testLog = logger.GetLog(); - Assert.Contains("// ### BenchmarkInnerClass method called ###" + Environment.NewLine, testLog); - Assert.Contains("// ### BenchmarkGenericInnerClass method called ###" + Environment.NewLine, testLog); - Assert.DoesNotContain("No benchmarks found", logger.GetLog()); + Assert.Equal(expected, GetCombinedStandardOutput(summary)); } public class Inner @@ -31,14 +37,14 @@ public class Inner [Benchmark] public Tuple BenchmarkInnerClass() { - Console.WriteLine("// ### BenchmarkInnerClass method called ###"); + Console.WriteLine(FirstExpectedMessage); return Tuple.Create(new Outer(), new Outer.Inner()); } [Benchmark] public Tuple> BenchmarkGenericInnerClass() { - Console.WriteLine("// ### BenchmarkGenericInnerClass method called ###"); + Console.WriteLine(SecondExpectedMessage); return Tuple.Create(new Outer(), new Outer.InnerGeneric()); } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs index 189a42137c..f7e4c2682d 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs @@ -17,11 +17,12 @@ public void Test() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); + var standardOutput = GetCombinedStandardOutput(summary); foreach (var param in new[] { 1, 2 }) - Assert.Contains($"// ### New Parameter {param} ###" + Environment.NewLine, logger.GetLog()); - Assert.DoesNotContain($"// ### New Parameter {default(int)} ###" + Environment.NewLine, logger.GetLog()); + Assert.Contains($"// ### New Parameter {param} ###", standardOutput); + Assert.DoesNotContain($"// ### New Parameter {default(int)} ###", standardOutput); } public class ParamsTestStaticProperty diff --git a/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs b/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs index 23a4a52222..a1d4a0fc1c 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs @@ -17,10 +17,13 @@ public void ParamsSupportPropertyWithPublicSetter() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); + var standardOutput = GetCombinedStandardOutput(summary); + foreach (var param in new[] { 1, 2 }) - Assert.Contains($"// ### New Parameter {param} ###" + Environment.NewLine, logger.GetLog()); - Assert.DoesNotContain($"// ### New Parameter {default(int)} ###" + Environment.NewLine, logger.GetLog()); + Assert.Contains($"// ### New Parameter {param} ###", standardOutput); + + Assert.DoesNotContain($"// ### New Parameter {default(int)} ###", standardOutput); } public class ParamsTestProperty @@ -72,11 +75,13 @@ public void ParamsSupportPublicFields() var logger = new OutputLogger(Output); var config = CreateSimpleConfig(logger); - CanExecute(config); + var summary = CanExecute(config); + var standardOutput = GetCombinedStandardOutput(summary); foreach (var param in new[] { 1, 2 }) - Assert.Contains($"// ### New Parameter {param} ###" + Environment.NewLine, logger.GetLog()); - Assert.DoesNotContain($"// ### New Parameter 0 ###" + Environment.NewLine, logger.GetLog()); + Assert.Contains($"// ### New Parameter {param} ###", standardOutput); + + Assert.DoesNotContain($"// ### New Parameter 0 ###", standardOutput); } public class ParamsTestField diff --git a/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs b/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs index 6be53ac3cf..e3a3edd8a5 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs @@ -40,10 +40,10 @@ public void RunStrategiesAreSupported() Assert.Equal(1, results.BenchmarksCases.Count(b => b.Job.Run.RunStrategy == RunStrategy.Throughput && b.Descriptor.WorkloadMethod.Name == "BenchmarkWithVoid")); Assert.Equal(1, results.BenchmarksCases.Count(b => b.Job.Run.RunStrategy == RunStrategy.Throughput && b.Descriptor.WorkloadMethod.Name == "BenchmarkWithReturnValue")); - string testLog = logger.GetLog(); - Assert.Contains("// ### Benchmark with void called ###", testLog); - Assert.Contains("// ### Benchmark with return value called ###", testLog); - Assert.DoesNotContain("No benchmarks found", logger.GetLog()); + var standardOutput = GetCombinedStandardOutput(results); + Assert.Contains("// ### Benchmark with void called ###", standardOutput); + Assert.Contains("// ### Benchmark with return value called ###", standardOutput); + Assert.DoesNotContain("No benchmarks found", standardOutput); } public class ModeBenchmarks diff --git a/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs b/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs index bd6352fe32..82afc50466 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs @@ -82,17 +82,18 @@ public void AllSetupAndCleanupMethodRunsForSpecificBenchmark() var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(logger, miniJob); - CanExecute(config); + var summary = CanExecute(config); + var standardOutput = GetCombinedStandardOutput(summary); Output.WriteLine(OutputDelimiter); Output.WriteLine(OutputDelimiter); Output.WriteLine(OutputDelimiter); - var firstActualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(FirstPrefix)).ToArray(); + var firstActualLogLines = standardOutput.Where(line => line.StartsWith(FirstPrefix)).ToArray(); foreach (string line in firstActualLogLines) Output.WriteLine(line); Assert.Equal(firstExpectedLogLines, firstActualLogLines); - var secondActualLogLines = logger.GetLog().Split('\r', '\n').Where(line => line.StartsWith(SecondPrefix)).ToArray(); + var secondActualLogLines = standardOutput.Where(line => line.StartsWith(SecondPrefix)).ToArray(); foreach (string line in secondActualLogLines) Output.WriteLine(line); Assert.Equal(secondExpectedLogLines, secondActualLogLines); From fcd55949dc2031202c788dd08b41256777e2c9c6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 08:29:38 +0200 Subject: [PATCH 11/22] remove tests covered by AllSetupAndCleanupTest --- .../GlobalCleanupAttributeTest.cs | 48 ------------------- .../GlobalSetupAttributeTest.cs | 48 ------------------- 2 files changed, 96 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs deleted file mode 100644 index fea28b46be..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTest.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class GlobalCleanupAttributeTest : BenchmarkTestExecutor - { - private const string GlobalCleanupCalled = "// ### GlobalCleanup called ###"; - private const string BenchmarkCalled = "// ### Benchmark called ###"; - - public GlobalCleanupAttributeTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void GlobalCleanupMethodRunsTest() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - - string[] expected = new string[] - { - BenchmarkCalled, - GlobalCleanupCalled - }; - - Assert.Equal(expected, GetSingleStandardOutput(summary)); - } - - public class GlobalCleanupAttributeBenchmarks - { - [Benchmark] - public void Benchmark() - { - Console.WriteLine(BenchmarkCalled); - } - - [GlobalCleanup] - public void GlobalCleanup() - { - Console.WriteLine(GlobalCleanupCalled); - } - } - } -} diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs deleted file mode 100644 index c1d6ec6da2..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTest.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class GlobalSetupAttributeTest : BenchmarkTestExecutor - { - private const string GlobalSetupCalled = "// ### GlobalSetup called ###"; - private const string BenchmarkCalled = "// ### Benchmark called ###"; - - public GlobalSetupAttributeTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void GlobalSetupMethodRunsTest() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - - string[] expected = new string[] - { - GlobalSetupCalled, - BenchmarkCalled - }; - - Assert.Equal(expected, GetSingleStandardOutput(summary)); - } - - public class GlobalSetupAttributeBenchmarks - { - [GlobalSetup] - public void GlobalSetup() - { - Console.WriteLine(GlobalSetupCalled); - } - - [Benchmark] - public void Benchmark() - { - Console.WriteLine(BenchmarkCalled); - } - } - } -} From b479681bb3e90aff610077c88ec2fde831251e53 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 08:32:33 +0200 Subject: [PATCH 12/22] remove tests covered by SetupAndCleanupTests --- .../GlobalCleanupAttributeTargetTest.cs | 97 ------------------- .../GlobalSetupAttributeTargetTest.cs | 97 ------------------- 2 files changed, 194 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs deleted file mode 100644 index 4e3129dc8a..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalCleanupAttributeTargetTest.cs +++ /dev/null @@ -1,97 +0,0 @@ -using System; -using System.Collections.Generic; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class GlobalCleanupAttributeTargetTest : BenchmarkTestExecutor - { - private const string BaselineGlobalCleanupCalled = "// ### Baseline GlobalCleanup called ###"; - private const string BaselineBenchmarkCalled = "// ### Baseline Benchmark called ###"; - private const string FirstGlobalCleanupCalled = "// ### First GlobalCleanup called ###"; - private const string FirstBenchmarkCalled = "// ### First Benchmark called ###"; - private const string SecondGlobalCleanupCalled = "// ### Second GlobalCleanup called ###"; - private const string SecondBenchmarkCalled = "// ### Second Benchmark called ###"; - - public GlobalCleanupAttributeTargetTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void GlobalCleanupTargetSpecificMethodTest() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - - IReadOnlyList standardOuput = GetCombinedStandardOutput(summary); - - string[] expected = new string[] - { - BaselineBenchmarkCalled, - BaselineGlobalCleanupCalled, - FirstBenchmarkCalled, - FirstGlobalCleanupCalled, - SecondBenchmarkCalled, - SecondGlobalCleanupCalled - }; - - Assert.Equal(expected, standardOuput); - } - - public class GlobalCleanupAttributeTargetBenchmarks - { - private int cleanupValue; - - [Benchmark(Baseline = true)] - public void BaselineBenchmark() - { - cleanupValue = -1; - - Console.WriteLine(BaselineBenchmarkCalled); - } - - [GlobalCleanup] - public void BaselineCleanup() - { - Assert.Equal(-1, cleanupValue); - - Console.WriteLine(BaselineGlobalCleanupCalled); - } - - [Benchmark] - public void Benchmark1() - { - cleanupValue = 1; - - Console.WriteLine(FirstBenchmarkCalled); - } - - [GlobalCleanup(Target = nameof(Benchmark1))] - public void GlobalCleanup1() - { - Assert.Equal(1, cleanupValue); - - Console.WriteLine(FirstGlobalCleanupCalled); - } - - [Benchmark] - public void Benchmark2() - { - cleanupValue = 2; - - Console.WriteLine(SecondBenchmarkCalled); - } - - [GlobalCleanup(Target = nameof(Benchmark2))] - public void GlobalCleanup2() - { - Assert.Equal(2, cleanupValue); - - Console.WriteLine(SecondGlobalCleanupCalled); - } - } - } -} diff --git a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs deleted file mode 100644 index 0e5020c7d8..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/GlobalSetupAttributeTargetTest.cs +++ /dev/null @@ -1,97 +0,0 @@ -using System; -using System.Collections.Generic; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class GlobalSetupAttributeTargetTest : BenchmarkTestExecutor - { - private const string BaselineGlobalSetupCalled = "// ### Baseline GlobalSetup called ###"; - private const string BaselineBenchmarkCalled = "// ### Baseline Benchmark called ###"; - private const string FirstGlobalSetupCalled = "// ### First GlobalSetup called ###"; - private const string FirstBenchmarkCalled = "// ### First Benchmark called ###"; - private const string SecondGlobalSetupCalled = "// ### Second GlobalSetup called ###"; - private const string SecondBenchmarkCalled = "// ### Second Benchmark called ###"; - - public GlobalSetupAttributeTargetTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void GlobalSetupTargetSpecificMethodTest() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - - IReadOnlyList standardOuput = GetCombinedStandardOutput(summary); - - string[] expected = new string[] - { - BaselineGlobalSetupCalled, - BaselineBenchmarkCalled, - FirstGlobalSetupCalled, - FirstBenchmarkCalled, - SecondGlobalSetupCalled, - SecondBenchmarkCalled, - }; - - Assert.Equal(expected, standardOuput); - } - - public class GlobalSetupAttributeTargetBenchmarks - { - private int setupValue; - - [GlobalSetup] - public void BaselineSetup() - { - setupValue = -1; - - Console.WriteLine(BaselineGlobalSetupCalled); - } - - [Benchmark(Baseline = true)] - public void BaselineBenchmark() - { - Assert.Equal(-1, setupValue); - - Console.WriteLine(BaselineBenchmarkCalled); - } - - [GlobalSetup(Target = nameof(Benchmark1))] - public void GlobalSetup1() - { - setupValue = 1; - - Console.WriteLine(FirstGlobalSetupCalled); - } - - [Benchmark] - public void Benchmark1() - { - Assert.Equal(1, setupValue); - - Console.WriteLine(FirstBenchmarkCalled); - } - - [GlobalSetup(Target = nameof(Benchmark2))] - public void GlobalSetup2() - { - setupValue = 2; - - Console.WriteLine(SecondGlobalSetupCalled); - } - - [Benchmark] - public void Benchmark2() - { - Assert.Equal(2, setupValue); - - Console.WriteLine(SecondBenchmarkCalled); - } - } - } -} From 72be7dd55f3f814cd21af3f784a8e2332bb2a0e2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 08:33:36 +0200 Subject: [PATCH 13/22] remove test that has been disabled for years --- .../PerformanceUnitTest.cs | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/PerformanceUnitTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/PerformanceUnitTest.cs b/tests/BenchmarkDotNet.IntegrationTests/PerformanceUnitTest.cs deleted file mode 100644 index 73224a087f..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/PerformanceUnitTest.cs +++ /dev/null @@ -1,74 +0,0 @@ -using System; -using System.Linq; -using System.Threading; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Configs; -using BenchmarkDotNet.Extensions; -using BenchmarkDotNet.Reports; -using BenchmarkDotNet.Running; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class PerformanceUnitTestRunner - { - private readonly ITestOutputHelper output; - - public PerformanceUnitTestRunner(ITestOutputHelper outputHelper) - { - output = outputHelper; - } - - // See also: https://github.com/dotnet/BenchmarkDotNet/issues/204 - [Fact(Skip = "This test fails on AppVeyor")] - public void Test() - { - var logger = new OutputLogger(output); - var config = DefaultConfig.Instance.AddLogger(logger); - var summary = BenchmarkRunner.Run(config); - - // Sanity checks, to be sure that the different benchmarks actually run - var testOutput = logger.GetLog(); - Assert.Contains("// ### Slow Benchmark called ###" + Environment.NewLine, testOutput); - Assert.Contains("// ### Fast Benchmark called ###" + Environment.NewLine, testOutput); - - // Check that slow benchmark is actually slower than the fast benchmark! - var slowBenchmarkRun = summary.GetRunsFor(r => r.SlowBenchmark()).First(); - var fastBenchmarkRun = summary.GetRunsFor(r => r.FastBenchmark()).First(); - Assert.True(slowBenchmarkRun.GetAverageTime() > fastBenchmarkRun.GetAverageTime(), - string.Format("Expected SlowBenchmark: {0:N2} ns to be MORE than FastBenchmark: {1:N2} ns", - slowBenchmarkRun.GetAverageTime().Nanoseconds, fastBenchmarkRun.GetAverageTime().Nanoseconds)); - Assert.True(slowBenchmarkRun.GetOpsPerSecond() < fastBenchmarkRun.GetOpsPerSecond(), - string.Format("Expected SlowBenchmark: {0:N2} Ops to be LESS than FastBenchmark: {1:N2} Ops", - slowBenchmarkRun.GetOpsPerSecond(), fastBenchmarkRun.GetOpsPerSecond())); - - // Whilst we're at it, let's do more specific Asserts as we know what the elapsed time should be - var slowBenchmarkReport = summary.GetReportFor(r => r.SlowBenchmark()); - var fastBenchmarkReport = summary.GetReportFor(r => r.FastBenchmark()); - foreach (var slowRun in slowBenchmarkReport.GetResultRuns()) - Assert.InRange(slowRun.GetAverageTime().Nanoseconds / 1000.0 / 1000.0, low: 98, high: 102); - foreach (var fastRun in fastBenchmarkReport.GetResultRuns()) - Assert.InRange(fastRun.GetAverageTime().Nanoseconds / 1000.0 / 1000.0, low: 14, high: 17); - } - } - - [Config(typeof(SingleRunFastConfig))] - public class PerformanceUnitTest - { - [Benchmark] - public void FastBenchmark() - { - Console.WriteLine("// ### Fast Benchmark called ###"); - Thread.Sleep(15); - } - - [Benchmark] - public void SlowBenchmark() - { - Console.WriteLine("// ### Slow Benchmark called ###"); - Thread.Sleep(100); - } - } -} From 6895d1eaf5f21af737141cc009184d2c8fc436d7 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 09:00:57 +0200 Subject: [PATCH 14/22] remove test covered by ArgumentsTests --- .../GenericBenchmarkTest.cs | 96 ------------------- 1 file changed, 96 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs b/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs deleted file mode 100644 index 373968ca03..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/GenericBenchmarkTest.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Linq; -using System.Reflection; -using System.Threading; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class GenericBenchmarkTest : BenchmarkTestExecutor - { - public GenericBenchmarkTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void GenericClassesAreSupported() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - var expected1 = $"// ### Benchmark: SerializationLibrary1, Type: {typeof(FlatClassBenchmark).Name} ###"; - Assert.Contains(expected1, GetCombinedStandardOutput(summary)); - - logger.ClearLog(); - summary = CanExecute(config); - var expected2 = $"// ### Benchmark: SerializationLibrary2, Type: {typeof(DoubleArrayBenchmark).Name} ###"; - Assert.Contains(expected2, GetCombinedStandardOutput(summary)); - } - } - - // From https://github.com/dotnet/BenchmarkDotNet/issues/44 - public abstract class AbstractBenchmark - { - private readonly T value; - private readonly Serializer1 serializer1 = new Serializer1(); - private readonly Serializer2 serializer2 = new Serializer2(); - - protected AbstractBenchmark() - { - value = CreateValue(); - } - - protected abstract T CreateValue(); - - [Benchmark] - public void SerializationLibrary1() - { - Thread.Sleep(10); - // Our direct type is BenchmarkDotNet.Autogenerated.Program, - // which inherits from BenchmarkDotNet.IntegrationTests.FlatClassBenchmark - Console.WriteLine($"// ### Benchmark: SerializationLibrary1, Type: {GetType().GetTypeInfo().BaseType.Name} ###"); - string text = serializer1.Serialize(value); - } - - [Benchmark] - public void SerializationLibrary2() - { - Thread.Sleep(10); - // Our direct type is BenchmarkDotNet.Autogenerated.Program, - // which inherits from BenchmarkDotNet.IntegrationTests.FlatClassBenchmark - Console.WriteLine($"// ### Benchmark: SerializationLibrary2, Type: {GetType().GetTypeInfo().BaseType.Name} ###"); - string text = serializer2.Serialize(value); - } - } - - [Config(typeof(SingleRunFastConfig))] - public class FlatClassBenchmark : AbstractBenchmark - { - protected override FlatClass CreateValue() => new FlatClass() { Number = 42, Text = "64", TimeStamp = DateTime.UtcNow }; - } - - [Config(typeof(SingleRunFastConfig))] - public class DoubleArrayBenchmark : AbstractBenchmark - { - protected override double[] CreateValue() => Enumerable.Repeat(42.0, 100 * 1000).ToArray(); - } - - public class FlatClass - { - public int Number { get; set; } - public string Text { get; set; } - public DateTime TimeStamp { get; set; } - } - - public class Serializer1 - { - public string Serialize(T value) => ""; - } - - public class Serializer2 - { - public string Serialize(T value) => ""; - } -} From 418ba0b87d32c224a30721325d964a1743a3c5e8 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 09:01:31 +0200 Subject: [PATCH 15/22] fold InnerClassTest into ValuesReturnedByBenchmarkTest --- .../InnerClassTest.cs | 63 ------------------- .../ValuesReturnedByBenchmarkTest.cs | 17 +++++ 2 files changed, 17 insertions(+), 63 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs b/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs deleted file mode 100644 index bc67717a53..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/InnerClassTest.cs +++ /dev/null @@ -1,63 +0,0 @@ -using System; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - // See https://github.com/dotnet/BenchmarkDotNet/issues/55 - // https://github.com/dotnet/BenchmarkDotNet/issues/59 is also related - public class InnerClassTest : BenchmarkTestExecutor - { - private const string FirstExpectedMessage = "// ### BenchmarkInnerClass method called ###"; - private const string SecondExpectedMessage = "// ### BenchmarkGenericInnerClass method called ###"; - - public InnerClassTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void InnerClassesAreSupported() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - - string[] expected = new string[] - { - FirstExpectedMessage, - SecondExpectedMessage - }; - - Assert.Equal(expected, GetCombinedStandardOutput(summary)); - } - - public class Inner - { - [Benchmark] - public Tuple BenchmarkInnerClass() - { - Console.WriteLine(FirstExpectedMessage); - return Tuple.Create(new Outer(), new Outer.Inner()); - } - - [Benchmark] - public Tuple> BenchmarkGenericInnerClass() - { - Console.WriteLine(SecondExpectedMessage); - return Tuple.Create(new Outer(), new Outer.InnerGeneric()); - } - } - } - - public class Outer - { - public class Inner - { - } - - public class InnerGeneric - { - } - } -} diff --git a/tests/BenchmarkDotNet.IntegrationTests/ValuesReturnedByBenchmarkTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ValuesReturnedByBenchmarkTest.cs index 6fd41689f5..f6a5d10428 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ValuesReturnedByBenchmarkTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ValuesReturnedByBenchmarkTest.cs @@ -122,6 +122,23 @@ public class Job { } [Benchmark] public nuint UnsignedNativeSizeInteger() => 0; + + [Benchmark] + public Tuple BenchmarkInnerClass() => Tuple.Create(new Outer(), new Outer.Inner()); + + [Benchmark] + public Tuple> BenchmarkGenericInnerClass() => Tuple.Create(new Outer(), new Outer.InnerGeneric()); + } + + public class Outer + { + public class Inner + { + } + + public class InnerGeneric + { + } } } } From ee8971c36cb38e06288629666796cf51756df9b2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 09:27:41 +0200 Subject: [PATCH 16/22] fold ParamsAttributeStaticTest into ParamsTests, simplify the tests --- .../ParamsAttributeStaticTest.cs | 149 ------------------ .../ParamsTests.cs | 103 ++++++------ 2 files changed, 53 insertions(+), 199 deletions(-) delete mode 100644 tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs deleted file mode 100644 index f7e4c2682d..0000000000 --- a/tests/BenchmarkDotNet.IntegrationTests/ParamsAttributeStaticTest.cs +++ /dev/null @@ -1,149 +0,0 @@ -using System; -using System.Collections.Generic; -using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; -using Xunit; -using Xunit.Abstractions; - -namespace BenchmarkDotNet.IntegrationTests -{ - public class ParamsTestStaticPropertyTest : BenchmarkTestExecutor - { - public ParamsTestStaticPropertyTest(ITestOutputHelper output) : base(output) { } - - [Fact] - public void Test() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); - var standardOutput = GetCombinedStandardOutput(summary); - - foreach (var param in new[] { 1, 2 }) - Assert.Contains($"// ### New Parameter {param} ###", standardOutput); - Assert.DoesNotContain($"// ### New Parameter {default(int)} ###", standardOutput); - } - - public class ParamsTestStaticProperty - { - /// - /// Deliberately made the Property "static" to ensure that Params also work okay in this scenario - /// - [Params(1, 2)] - public static int StaticParamProperty { get; set; } - - private static HashSet collectedParams = new HashSet(); - - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(StaticParamProperty) == false) - { - Console.WriteLine($"// ### New Parameter {StaticParamProperty} ###"); - collectedParams.Add(StaticParamProperty); - } - } - } - } - - public class ParamsTestStaticPrivatePropertyError : BenchmarkTestExecutor - { - public ParamsTestStaticPrivatePropertyError(ITestOutputHelper output) : base(output) { } - - /// - /// Deliberately made the Property "static" to ensure that Params also work okay in this scenario - /// - [Params(1, 2)] - public static int StaticParamProperty { get; private set; } - - private static HashSet collectedParams = new HashSet(); - - [Fact] - public void Test() - { - // System.InvalidOperationException : Property "StaticParamProperty" must be public and writable if it has the [Params(..)] attribute applied to it - Assert.Throws(() => CanExecute()); - } - -#pragma warning disable xUnit1013 // Public method should be marked as test - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(StaticParamProperty) == false) - { - Console.WriteLine($"// ### New Parameter {StaticParamProperty} ###"); - collectedParams.Add(StaticParamProperty); - } - } -#pragma warning restore xUnit1013 // Public method should be marked as test - } - - // Deliberately made everything "static" (as well as using a Field) to ensure that Params also work okay in this scenario - public class ParamsTestStaticFieldTest : BenchmarkTestExecutor - { - public ParamsTestStaticFieldTest(ITestOutputHelper output) : base(output) - { - } - - [Fact] - public void Test() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - CanExecute(config); - - foreach (var param in new[] { 1, 2 }) - Assert.Contains($"// ### New Parameter {param} ###" + Environment.NewLine, logger.GetLog()); - Assert.DoesNotContain($"// ### New Parameter 0 ###" + Environment.NewLine, logger.GetLog()); - } - - public class ParamsTestStaticField - { - [Params(1, 2)] - public static int StaticParamField = 0; - - private static HashSet collectedParams = new HashSet(); - - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(StaticParamField) == false) - { - Console.WriteLine($"// ### New Parameter {StaticParamField} ###"); - collectedParams.Add(StaticParamField); - } - } - } - } - - // Deliberately made everything "static" (as well as using a Field) to ensure that Params also work okay in this scenario - [Config(typeof(SingleRunFastConfig))] - public class ParamsTestStaticPrivateFieldError - { - [Params(1, 2)] - private static int StaticParamField = 0; - - private static HashSet collectedParams = new HashSet(); - - [Fact] - public static void Test() - { - // System.InvalidOperationException : Field "StaticParamField" must be public if it has the [Params(..)] attribute applied to it - Assert.Throws(() => new BenchmarkTestExecutor().CanExecute()); - } - -#pragma warning disable xUnit1013 // Public method should be marked as test - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(StaticParamField) == false) - { - Console.WriteLine($"// ### New Parameter {StaticParamField} ###"); - collectedParams.Add(StaticParamField); - } - } -#pragma warning restore xUnit1013 // Public method should be marked as test - } -} diff --git a/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs b/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs index a1d4a0fc1c..d610793bf1 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ParamsTests.cs @@ -1,7 +1,5 @@ using System; -using System.Collections.Generic; using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; using Xunit; using Xunit.Abstractions; @@ -14,10 +12,7 @@ public ParamsTests(ITestOutputHelper output) : base(output) { } [Fact] public void ParamsSupportPropertyWithPublicSetter() { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); + var summary = CanExecute(); var standardOutput = GetCombinedStandardOutput(summary); foreach (var param in new[] { 1, 2 }) @@ -31,17 +26,8 @@ public class ParamsTestProperty [Params(1, 2)] public int ParamProperty { get; set; } - private HashSet collectedParams = new HashSet(); - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(ParamProperty) == false) - { - Console.WriteLine($"// ### New Parameter {ParamProperty} ###"); - collectedParams.Add(ParamProperty); - } - } + public void Benchmark() => Console.WriteLine($"// ### New Parameter {ParamProperty} ###"); } [Fact] @@ -56,26 +42,14 @@ public class ParamsTestPrivatePropertyError [Params(1, 2)] public int ParamProperty { get; private set; } - private HashSet collectedParams = new HashSet(); - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(ParamProperty) == false) - { - Console.WriteLine($"// ### New Parameter {ParamProperty} ###"); - collectedParams.Add(ParamProperty); - } - } + public void Benchmark() => Console.WriteLine($"// ### New Parameter {ParamProperty} ###"); } [Fact] public void ParamsSupportPublicFields() { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - var summary = CanExecute(config); + var summary = CanExecute(); var standardOutput = GetCombinedStandardOutput(summary); foreach (var param in new[] { 1, 2 }) @@ -89,17 +63,8 @@ public class ParamsTestField [Params(1, 2)] public int ParamField = 0; - private HashSet collectedParams = new HashSet(); - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(ParamField) == false) - { - Console.WriteLine($"// ### New Parameter {ParamField} ###"); - collectedParams.Add(ParamField); - } - } + public void Benchmark() => Console.WriteLine($"// ### New Parameter {ParamField} ###"); } [Fact] @@ -114,17 +79,8 @@ public class ParamsTestPrivateFieldError [Params(1, 2)] private int ParamField = 0; - private HashSet collectedParams = new HashSet(); - [Benchmark] - public void Benchmark() - { - if (collectedParams.Contains(ParamField) == false) - { - Console.WriteLine($"// ### New Parameter {ParamField} ###"); - collectedParams.Add(ParamField); - } - } + public void Benchmark() => Console.WriteLine($"// ### New Parameter {ParamField} ###"); } public enum NestedOne @@ -221,5 +177,52 @@ public void AcceptingArray() throw new InvalidOperationException($"Incorrect array element at index {i}, was {Array[i]} instead of {i}"); } } + + [Fact] + public void StaticFieldsAndPropertiesCanBeParams() => CanExecute(); + + public class WithStaticParams + { + [Params(1)] + public static int StaticParamField = 0; + + [Params(2)] + public static int StaticParamProperty { get; set; } = 0; + + [Benchmark] + public void Test() + { + if (StaticParamField != 1) + throw new ArgumentException($"{nameof(StaticParamField)} has wrong value: {StaticParamField}!"); + if (StaticParamProperty != 2) + throw new ArgumentException($"{nameof(StaticParamProperty)} has wrong value: {StaticParamProperty}!"); + } + } + + [Fact] + public void ParamsPropertiesMustHavePublicSetter() + => Assert.Throws(() => CanExecute()); + + public class WithStaticParamsPropertyWithNoPublicSetter + { + [Params(3)] + public static int StaticParamProperty { get; private set; } + + [Benchmark] + public int Benchmark() => StaticParamProperty; + } + + [Fact] + public void ParamsFieldsMustBePublic() + => Assert.Throws(() => CanExecute()); + + public class WithStaticPrivateParamsField + { + [Params(4)] + private static int StaticParamField = 0; + + [Benchmark] + public int Benchmark() => StaticParamField; + } } } \ No newline at end of file From cc6031d6c0cc0a32b10b96d28247e2051f9487e4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 10:01:00 +0200 Subject: [PATCH 17/22] CanExecute ensures that at least one benchmark has executed successfully, there is no need to write to std out --- .../DryRunTests.cs | 30 ++++--- .../JitRuntimeValidationTest.cs | 81 ++++++++----------- .../ProcessorArchitectureTest.cs | 35 ++------ .../RunStrategyTests.cs | 49 ++++------- 4 files changed, 73 insertions(+), 122 deletions(-) diff --git a/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs b/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs index 31a3e43bd8..b9ca511236 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/DryRunTests.cs @@ -1,7 +1,8 @@ using System; +using System.Linq; using System.Threading; using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Tests.Loggers; +using BenchmarkDotNet.Engines; using Xunit; using Xunit.Abstractions; @@ -12,7 +13,7 @@ public class DryRunTests : BenchmarkTestExecutor public DryRunTests(ITestOutputHelper output) : base(output) { } [Fact] - public void WelchTTest() => CanExecute(CreateSimpleConfig()); + public void WelchTTest() => CanExecute(); [DryJob, StatisticalTestColumn] public class WelchTTestBench @@ -24,19 +25,22 @@ public class WelchTTestBench public void B() => Thread.Sleep(10); } - private const string CounterPrefix = "// Counter = "; - [Fact] public void ColdStart() { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); + var summary = CanExecute(); + + var report = summary.Reports.Single(); - var summary = CanExecute(config); - var standardOutput = GetSingleStandardOutput(summary); + Assert.Equal(2, report.AllMeasurements.Count); - Assert.Contains($"{CounterPrefix}1", standardOutput); - Assert.DoesNotContain($"{CounterPrefix}2", standardOutput); + foreach (var measurement in report.AllMeasurements) + { + Assert.Equal(1, measurement.LaunchIndex); + Assert.Equal(1, measurement.IterationIndex); + Assert.Equal(IterationMode.Workload, measurement.IterationMode); + Assert.True(measurement.IterationStage is IterationStage.Actual or IterationStage.Result); + } } [DryJob] @@ -47,8 +51,10 @@ public class ColdStartBench [Benchmark] public void Foo() { - counter++; - Console.WriteLine($"{CounterPrefix}{counter}"); + if (++counter > 1) + { + throw new InvalidOperationException("Benchmark was executed more than once"); + } } } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs b/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs index a87e43997b..9a64e40190 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs @@ -1,10 +1,9 @@ -using System; +using System.Linq; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Columns; using BenchmarkDotNet.Configs; using BenchmarkDotNet.Environments; using BenchmarkDotNet.Jobs; -using BenchmarkDotNet.Running; using BenchmarkDotNet.Tests.Loggers; using BenchmarkDotNet.Tests.XUnit; using Xunit; @@ -12,79 +11,63 @@ namespace BenchmarkDotNet.IntegrationTests { - public class JitRuntimeValidationTest + public class JitRuntimeValidationTest : BenchmarkTestExecutor { - protected readonly ITestOutputHelper Output; + public JitRuntimeValidationTest(ITestOutputHelper output) : base(output) { } - private class PlatformConfig : ManualConfig - { - public PlatformConfig(Runtime runtime, Jit jit, Platform platform) - { - AddJob(new Job(Job.Dry, new EnvironmentMode() - { - Runtime = runtime, - Jit = jit, - Platform = platform - })); - } - } - - private const string OkCaption = "// OkCaption"; - // private const string LegacyJitNotAvailableForMono = "// ERROR: LegacyJIT is requested but it is not available for Mono"; +// private const string LegacyJitNotAvailableForMono = "// ERROR: LegacyJIT is requested but it is not available for Mono"; private const string RyuJitNotAvailable = "// ERROR: RyuJIT is requested but it is not available in current environment"; private const string ToolchainSupportsOnlyRyuJit = "Currently dotnet cli toolchain supports only RyuJit"; - public JitRuntimeValidationTest(ITestOutputHelper outputHelper) - { - Output = outputHelper; - } - [TheoryWindowsOnly("CLR is a valid job only on Windows")] - [InlineData(Jit.LegacyJit, Platform.X86, OkCaption)] - [InlineData(Jit.LegacyJit, Platform.X64, OkCaption)] + [InlineData(Jit.LegacyJit, Platform.X86, null)] + [InlineData(Jit.LegacyJit, Platform.X64, null)] [InlineData(Jit.RyuJit, Platform.X86, RyuJitNotAvailable)] - [InlineData(Jit.RyuJit, Platform.X64, OkCaption)] - public void CheckClrOnWindows(Jit jit, Platform platform, string expectedText) + [InlineData(Jit.RyuJit, Platform.X64, null)] + public void CheckClrOnWindows(Jit jit, Platform platform, string errorMessage) { - Verify(ClrRuntime.Net462, jit, platform, expectedText); + Verify(ClrRuntime.Net462, jit, platform, errorMessage); } -// [TheoryWindowsOnly("CLR is a valid job only on Windows")] -// [InlineData(Jit.LegacyJit, Platform.X86, LegacyJitNotAvailableForMono)] -// [InlineData(Jit.LegacyJit, Platform.X64, LegacyJitNotAvailableForMono)] -// [InlineData(Jit.RyuJit, Platform.X86, RyuJitNotAvailable)] -// [InlineData(Jit.RyuJit, Platform.X64, RyuJitNotAvailable)] -// public void CheckMono(Jit jit, Platform platform, string expectedText) -// { -// Verify(Runtime.Mono, jit, platform, expectedText); -// } +// [TheoryWindowsOnly("CLR is a valid job only on Windows")] +// [InlineData(Jit.LegacyJit, Platform.X86, LegacyJitNotAvailableForMono)] +// [InlineData(Jit.LegacyJit, Platform.X64, LegacyJitNotAvailableForMono)] +// [InlineData(Jit.RyuJit, Platform.X86, RyuJitNotAvailable)] +// [InlineData(Jit.RyuJit, Platform.X64, RyuJitNotAvailable)] +// public void CheckMono(Jit jit, Platform platform, string errorMessage) +// { +// Verify(Runtime.Mono, jit, platform, errorMessage); +// } [Theory] [InlineData(Jit.LegacyJit, Platform.X86, ToolchainSupportsOnlyRyuJit)] [InlineData(Jit.LegacyJit, Platform.X64, ToolchainSupportsOnlyRyuJit)] - [InlineData(Jit.RyuJit, Platform.X64, OkCaption)] - public void CheckCore(Jit jit, Platform platform, string expectedText) + [InlineData(Jit.RyuJit, Platform.X64, null)] + public void CheckCore(Jit jit, Platform platform, string errorMessage) { - Verify(CoreRuntime.Core60, jit, platform, expectedText); + Verify(CoreRuntime.Core60, jit, platform, errorMessage); } - private void Verify(Runtime runtime, Jit jit, Platform platform, string expectedText) + private void Verify(Runtime runtime, Jit jit, Platform platform, string errorMessage) { var logger = new OutputLogger(Output); - var config = new PlatformConfig(runtime, jit, platform).AddLogger(logger).AddColumnProvider(DefaultColumnProviders.Instance); + var config = ManualConfig.CreateEmpty() + .AddJob(Job.Dry.WithPlatform(platform).WithJit(jit).WithRuntime(runtime)) + .AddLogger(logger) + .AddColumnProvider(DefaultColumnProviders.Instance); - BenchmarkRunner.Run(new[] { BenchmarkConverter.TypeToBenchmarks(typeof(TestBenchmark), config) }); + var summary = CanExecute(config, fullValidation: errorMessage is null); - Assert.Contains(expectedText, logger.GetLog()); + if (errorMessage is not null) + { + Assert.Contains(errorMessage, logger.GetLog()); + } } public class TestBenchmark { [Benchmark] - public void Benchmark() - { - Console.WriteLine(OkCaption); - } + public void Benchmark() { } } } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs index a763a9b9a8..3726316207 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs @@ -11,12 +11,6 @@ namespace BenchmarkDotNet.IntegrationTests { public class ProcessorArchitectureTest : BenchmarkTestExecutor { - private const string X86FailedCaption = "// x86FAILED"; - private const string X64FailedCaption = "// x64FAILED"; - private const string AnyCpuOkCaption = "// AnyCpuOkCaption"; - private const string HostPlatformOkCaption = "// HostPlatformOkCaption"; - private const string BenchmarkNotFound = "// There are no benchmarks found"; - public ProcessorArchitectureTest(ITestOutputHelper outputHelper) : base(outputHelper) { } @@ -25,13 +19,13 @@ public ProcessorArchitectureTest(ITestOutputHelper outputHelper) : base(outputHe public void SpecifiedProcessorArchitectureMustBeRespected() { #if NETFRAMEWORK // dotnet cli does not support x86 compilation so far, so I disable this test - Verify(Platform.X86, typeof(X86Benchmark), X86FailedCaption); + Verify(Platform.X86, typeof(X86Benchmark)); #endif - Verify(Platform.X64, typeof(X64Benchmark), X64FailedCaption); - Verify(Platform.AnyCpu, typeof(AnyCpuBenchmark), "nvm"); + Verify(Platform.X64, typeof(X64Benchmark)); + Verify(Platform.AnyCpu, typeof(AnyCpuBenchmark)); } - private void Verify(Platform platform, Type benchmark, string failureText) + private void Verify(Platform platform, Type benchmark) { var logger = new OutputLogger(Output); @@ -39,11 +33,8 @@ private void Verify(Platform platform, Type benchmark, string failureText) .AddJob(Job.Dry.WithPlatform(platform)) .AddLogger(logger); // make sure we get an output in the TestRunner log - CanExecute(benchmark, config); - - var testLog = logger.GetLog(); - Assert.DoesNotContain(failureText, testLog); - Assert.DoesNotContain(BenchmarkNotFound, testLog); + // CanExecute ensures that at least one benchmark has executed successfully + CanExecute(benchmark, config, fullValidation: true); } public class X86Benchmark @@ -53,7 +44,7 @@ public void _32Bit() { if (IntPtr.Size != 4) { - throw new InvalidOperationException(X86FailedCaption); + throw new InvalidOperationException("32 bit failed!"); } } } @@ -65,7 +56,7 @@ public void _64Bit() { if (IntPtr.Size != 8) { - throw new InvalidOperationException(X64FailedCaption); + throw new InvalidOperationException("64 bit failed!"); } } } @@ -75,16 +66,6 @@ public class AnyCpuBenchmark [Benchmark] public void AnyCpu() { - Console.WriteLine(AnyCpuOkCaption); - } - } - - public class HostBenchmark - { - [Benchmark] - public void Host() - { - Console.WriteLine(HostPlatformOkCaption); } } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs b/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs index e3a3edd8a5..7a5ac84a96 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/RunStrategyTests.cs @@ -19,10 +19,9 @@ public RunStrategyTests(ITestOutputHelper output) : base(output) { } [Fact] public void RunStrategiesAreSupported() { - var logger = new OutputLogger(Output); var config = ManualConfig.CreateEmpty() .AddColumnProvider(DefaultColumnProviders.Instance) - .AddLogger(logger) + .AddLogger(new OutputLogger(Output)) .AddJob(new Job(Job.Dry) { Run = { RunStrategy = RunStrategy.ColdStart } }) .AddJob(new Job(Job.Dry) { Run = { RunStrategy = RunStrategy.Monitoring } }) .AddJob(new Job(Job.Dry) { Run = { RunStrategy = RunStrategy.Throughput } }); @@ -40,45 +39,27 @@ public void RunStrategiesAreSupported() Assert.Equal(1, results.BenchmarksCases.Count(b => b.Job.Run.RunStrategy == RunStrategy.Throughput && b.Descriptor.WorkloadMethod.Name == "BenchmarkWithVoid")); Assert.Equal(1, results.BenchmarksCases.Count(b => b.Job.Run.RunStrategy == RunStrategy.Throughput && b.Descriptor.WorkloadMethod.Name == "BenchmarkWithReturnValue")); - var standardOutput = GetCombinedStandardOutput(results); - Assert.Contains("// ### Benchmark with void called ###", standardOutput); - Assert.Contains("// ### Benchmark with return value called ###", standardOutput); - Assert.DoesNotContain("No benchmarks found", standardOutput); + Assert.Equal(6, results.Reports.Length); + + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.ColdStart && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithVoid")); + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.ColdStart && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithReturnValue")); + + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.Monitoring && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithVoid")); + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.Monitoring && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithReturnValue")); + + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.Throughput && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithVoid")); + Assert.Single(results.Reports.Where(r => r.BenchmarkCase.Job.Run.RunStrategy == RunStrategy.Throughput && r.BenchmarkCase.Descriptor.WorkloadMethod.Name == "BenchmarkWithReturnValue")); + + Assert.True(results.Reports.All(r => r.AllMeasurements.Any())); } public class ModeBenchmarks { - public bool FirstTime { get; set; } - - [GlobalSetup] - public void GlobalSetup() - { - // Ensure we only print the diagnostic messages once per run in the tests, otherwise it fills up the output log!! - FirstTime = true; - } - [Benchmark] - public void BenchmarkWithVoid() - { - Thread.Sleep(10); - if (FirstTime) - { - Console.WriteLine("// ### Benchmark with void called ###"); - FirstTime = false; - } - } + public void BenchmarkWithVoid() { } [Benchmark] - public string BenchmarkWithReturnValue() - { - Thread.Sleep(10); - if (FirstTime) - { - Console.WriteLine("// ### Benchmark with return value called ###"); - FirstTime = false; - } - return "okay"; - } + public string BenchmarkWithReturnValue() => "okay"; } } } \ No newline at end of file From 1cb4a85fe32c484d43a228bc909476922fc728b3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 15:12:39 +0200 Subject: [PATCH 18/22] lock OutputLogger to see if it helps with flaky tests --- .../JitRuntimeValidationTest.cs | 2 +- tests/BenchmarkDotNet.Tests/Loggers/OutputLogger.cs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs b/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs index 9a64e40190..92b6b85106 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/JitRuntimeValidationTest.cs @@ -56,7 +56,7 @@ private void Verify(Runtime runtime, Jit jit, Platform platform, string errorMes .AddLogger(logger) .AddColumnProvider(DefaultColumnProviders.Instance); - var summary = CanExecute(config, fullValidation: errorMessage is null); + CanExecute(config, fullValidation: errorMessage is null); if (errorMessage is not null) { diff --git a/tests/BenchmarkDotNet.Tests/Loggers/OutputLogger.cs b/tests/BenchmarkDotNet.Tests/Loggers/OutputLogger.cs index 6cdd64008b..4cffd8dad4 100644 --- a/tests/BenchmarkDotNet.Tests/Loggers/OutputLogger.cs +++ b/tests/BenchmarkDotNet.Tests/Loggers/OutputLogger.cs @@ -1,4 +1,5 @@ using System; +using System.Runtime.CompilerServices; using BenchmarkDotNet.Loggers; using Xunit.Abstractions; @@ -14,12 +15,14 @@ public OutputLogger(ITestOutputHelper testOutputHelper) this.testOutputHelper = testOutputHelper ?? throw new ArgumentNullException(nameof(testOutputHelper)); } + [MethodImpl(MethodImplOptions.Synchronized)] public override void Write(LogKind logKind, string text) { currentLine += text; base.Write(logKind, text); } + [MethodImpl(MethodImplOptions.Synchronized)] public override void WriteLine() { testOutputHelper.WriteLine(currentLine); @@ -27,6 +30,7 @@ public override void WriteLine() base.WriteLine(); } + [MethodImpl(MethodImplOptions.Synchronized)] public override void WriteLine(LogKind logKind, string text) { testOutputHelper.WriteLine(currentLine + text); From 581098a66d0e79f4d545cfb2197534f252768e0d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Sep 2022 17:58:53 +0200 Subject: [PATCH 19/22] use CancelRead when process does not exit on time, disable FileStream buffering --- src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs | 2 +- src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs | 2 +- src/BenchmarkDotNet/Toolchains/Executor.cs | 2 +- src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs b/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs index f71aefaefc..de0eb298b8 100644 --- a/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs +++ b/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs @@ -24,7 +24,7 @@ public AnonymousPipesHost(string writHandle, string readHandle) } private Stream OpenAnonymousPipe(string fileHandle, FileAccess access) - => new FileStream(new SafeFileHandle(new IntPtr(int.Parse(fileHandle)), ownsHandle: true), access); + => new FileStream(new SafeFileHandle(new IntPtr(int.Parse(fileHandle)), ownsHandle: true), access, bufferSize: 1); public void Dispose() { diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 260b782345..ba2ffc9a0e 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -102,7 +102,7 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, { logger.WriteLineInfo($"// The benchmarking process did not quit within {ExecuteParameters.ProcessExitTimeout.TotalSeconds} seconds, it's going to get force killed now."); - processOutputReader.StopRead(); + processOutputReader.CancelRead(); consoleExitHandler.KillProcessTree(); } else diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 44352e477b..4f6f1a79fa 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -83,7 +83,7 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas { logger.WriteLineInfo("// The benchmarking process did not quit on time, it's going to get force killed now."); - processOutputReader.StopRead(); + processOutputReader.CancelRead(); consoleExitHandler.KillProcessTree(); } else diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs index 41417f0675..74b55a1e4a 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs @@ -93,7 +93,7 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas { logger.WriteLineInfo("// The benchmarking process did not finish within 10 minutes, it's going to get force killed now."); - processOutputReader.StopRead(); + processOutputReader.CancelRead(); consoleExitHandler.KillProcessTree(); } else From 691d11a81b53c619559e2dd5b37f47a6ede7b649 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Sep 2022 12:21:24 +0200 Subject: [PATCH 20/22] start reading the output as soon as process is started --- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 11 +++++------ src/BenchmarkDotNet/Toolchains/Executor.cs | 3 +-- .../Toolchains/MonoWasm/WasmExecutor.cs | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index ba2ffc9a0e..403f38bf02 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -80,13 +80,14 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, using (ConsoleExitHandler consoleExitHandler = new (process, logger)) using (AsyncProcessOutputReader processOutputReader = new (process, logOutput: true, logger, readStandardError: false)) { - var loggerWithDiagnoser = new Broker(logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); + Broker broker = new (logger, process, diagnoser, benchmarkCase, benchmarkId, inputFromBenchmark, acknowledgments); logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); diagnoser?.Handle(HostSignal.BeforeProcessStart, new DiagnoserActionParameters(process, benchmarkCase, benchmarkId)); process.Start(); + processOutputReader.BeginRead(); process.EnsureHighPriority(logger); if (benchmarkCase.Job.Environment.HasValue(EnvironmentMode.AffinityCharacteristic)) @@ -94,9 +95,7 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, process.TrySetAffinity(benchmarkCase.Job.Environment.Affinity, logger); } - processOutputReader.BeginRead(); - - loggerWithDiagnoser.ProcessData(); + broker.ProcessData(); if (!process.WaitForExit(milliseconds: (int)ExecuteParameters.ProcessExitTimeout.TotalMilliseconds)) { @@ -113,8 +112,8 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, return new ExecuteResult(true, process.HasExited ? process.ExitCode : null, process.Id, - loggerWithDiagnoser.Results, - loggerWithDiagnoser.PrefixedOutput, + broker.Results, + broker.PrefixedOutput, processOutputReader.GetOutputLines(), launchIndex); } diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 4f6f1a79fa..8eb0576a76 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -68,6 +68,7 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); process.Start(); + processOutputReader.BeginRead(); process.EnsureHighPriority(logger); if (benchmarkCase.Job.Environment.HasValue(EnvironmentMode.AffinityCharacteristic)) @@ -75,8 +76,6 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas process.TrySetAffinity(benchmarkCase.Job.Environment.Affinity, logger); } - processOutputReader.BeginRead(); - broker.ProcessData(); if (!process.WaitForExit(milliseconds: (int)ExecuteParameters.ProcessExitTimeout.TotalMilliseconds)) diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs index 74b55a1e4a..024c8e4242 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmExecutor.cs @@ -80,7 +80,6 @@ private static ExecuteResult Execute(Process process, BenchmarkCase benchmarkCas logger.WriteLineInfo($"// Execute: {process.StartInfo.FileName} {process.StartInfo.Arguments} in {process.StartInfo.WorkingDirectory}"); process.Start(); - processOutputReader.BeginRead(); process.EnsureHighPriority(logger); From 53943833abda7030ee555381cdc72d73ee9d0000 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Sep 2022 12:56:56 +0200 Subject: [PATCH 21/22] Before the last signal is reported and the benchmark process exits, add an artificial sleep to increase the chance of host process reading all std output. --- src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs b/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs index de0eb298b8..f1d5390a9f 100644 --- a/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs +++ b/src/BenchmarkDotNet/Engines/AnonymousPipesHost.cs @@ -40,6 +40,13 @@ public void Dispose() public void SendSignal(HostSignal hostSignal) { + if (hostSignal == HostSignal.AfterAll) + { + // Before the last signal is reported and the benchmark process exits, + // add an artificial sleep to increase the chance of host process reading all std output. + System.Threading.Thread.Sleep(1); + } + WriteLine(Engine.Signals.ToMessage(hostSignal)); // read the response from Parent process, make the communication blocking From 3906381eaa7a7a28dc8c68eb7582626c858b68e4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Sep 2022 14:06:50 +0200 Subject: [PATCH 22/22] FSharp tests: don't use standard output for benchmark failure verification --- .../Program.fs | 8 ++------ .../AllSetupAndCleanupTest.cs | 18 ++++++------------ .../FSharpTests.cs | 16 ++-------------- .../PowerManagementApplierTests.cs | 6 ++---- .../PriorityTests.cs | 3 +-- .../ProcessorArchitectureTest.cs | 4 +--- .../RoslynToolchainTest.cs | 3 +-- .../SetupAndCleanupTests.cs | 3 +-- 8 files changed, 16 insertions(+), 45 deletions(-) diff --git a/tests/BenchmarkDotNet.IntegrationTests.FSharp/Program.fs b/tests/BenchmarkDotNet.IntegrationTests.FSharp/Program.fs index b90d780ce5..e8192e4c62 100644 --- a/tests/BenchmarkDotNet.IntegrationTests.FSharp/Program.fs +++ b/tests/BenchmarkDotNet.IntegrationTests.FSharp/Program.fs @@ -28,13 +28,9 @@ type Db() = type TestEnum = | A = 0 | B = 1 | C = 2 type EnumParamsTest() = - let mutable collectedParams = HashSet() - - [] + [] member val EnumParamValue = TestEnum.A with get, set [] member this.Benchmark() = - if not <| collectedParams.Contains(this.EnumParamValue) then - printfn "// ### New Parameter %A ###" this.EnumParamValue - collectedParams.Add(this.EnumParamValue) |> ignore \ No newline at end of file + if not (this.EnumParamValue = TestEnum.B) then failwith "Invalid Params value assigned" diff --git a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs index 5bb5e225a2..6975a01401 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs @@ -52,9 +52,8 @@ private static string[] GetActualLogLines(Summary summary) [Fact] public void AllSetupAndCleanupMethodRunsTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); @@ -88,9 +87,8 @@ public class AllSetupAndCleanupAttributeBenchmarks [Fact] public void AllSetupAndCleanupMethodRunsAsyncTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); @@ -124,9 +122,8 @@ public class AllSetupAndCleanupAttributeBenchmarksAsync [Fact] public void AllSetupAndCleanupMethodRunsAsyncTaskSetupTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); @@ -160,9 +157,8 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncTaskSetup [Fact] public void AllSetupAndCleanupMethodRunsAsyncGenericTaskSetupTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); @@ -206,9 +202,8 @@ public async Task GlobalCleanup() [Fact] public void AllSetupAndCleanupMethodRunsAsyncValueTaskSetupTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); @@ -242,9 +237,8 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncValueTaskSetup [FactNotGitHubActionsWindows] public void AllSetupAndCleanupMethodRunsAsyncGenericValueTaskSetupTest() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); diff --git a/tests/BenchmarkDotNet.IntegrationTests/FSharpTests.cs b/tests/BenchmarkDotNet.IntegrationTests/FSharpTests.cs index 89bd3e29ca..2b0928e9b4 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/FSharpTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/FSharpTests.cs @@ -1,8 +1,5 @@ -using System; -using Xunit; +using Xunit; using Xunit.Abstractions; - -using BenchmarkDotNet.Tests.Loggers; using static FSharpBenchmarks; namespace BenchmarkDotNet.IntegrationTests @@ -12,15 +9,6 @@ public class FSharpTests : BenchmarkTestExecutor public FSharpTests(ITestOutputHelper output) : base(output) { } [Fact] - public void ParamsSupportFSharpEnums() - { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); - - CanExecute(config); - foreach (var param in new[] { TestEnum.A, TestEnum.B }) - Assert.Contains($"// ### New Parameter {param} ###" + Environment.NewLine, logger.GetLog()); - Assert.DoesNotContain($"// ### New Parameter {TestEnum.C} ###" + Environment.NewLine, logger.GetLog()); - } + public void ParamsSupportFSharpEnums() => CanExecute(); } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/PowerManagementApplierTests.cs b/tests/BenchmarkDotNet.IntegrationTests/PowerManagementApplierTests.cs index de129c9ac1..46bc67d655 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/PowerManagementApplierTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/PowerManagementApplierTests.cs @@ -18,8 +18,7 @@ public PowerManagementApplierTests(ITestOutputHelper output) : base(output) { } public void TestSettingAndRevertingBackGuid() { var userPlan = PowerManagementHelper.CurrentPlan; - var logger = new OutputLogger(Output); - var powerManagementApplier = new PowerManagementApplier(logger); + var powerManagementApplier = new PowerManagementApplier(new OutputLogger(Output)); powerManagementApplier.ApplyPerformancePlan(PowerManagementApplier.Map(PowerPlan.HighPerformance)); @@ -34,8 +33,7 @@ public void TestSettingAndRevertingBackGuid() public void TestPowerPlanShouldNotChange() { var userPlan = PowerManagementHelper.CurrentPlan; - var logger = new OutputLogger(Output); - var powerManagementApplier = new PowerManagementApplier(logger); + var powerManagementApplier = new PowerManagementApplier(new OutputLogger(Output)); powerManagementApplier.ApplyPerformancePlan(PowerManagementApplier.Map(PowerPlan.UserPowerPlan)); diff --git a/tests/BenchmarkDotNet.IntegrationTests/PriorityTests.cs b/tests/BenchmarkDotNet.IntegrationTests/PriorityTests.cs index 4976362314..47ba0b4089 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/PriorityTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/PriorityTests.cs @@ -14,8 +14,7 @@ public PriorityTests(ITestOutputHelper output) : base(output) { } [Fact] public void ParamsSupportPropertyWithPublicSetter() { - var logger = new OutputLogger(Output); - var config = CreateSimpleConfig(logger); + var config = CreateSimpleConfig(); var summary = CanExecute(config); var columns = summary.Table.Columns; diff --git a/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs b/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs index 3726316207..ea942ef8f5 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/ProcessorArchitectureTest.cs @@ -27,11 +27,9 @@ public void SpecifiedProcessorArchitectureMustBeRespected() private void Verify(Platform platform, Type benchmark) { - var logger = new OutputLogger(Output); - var config = ManualConfig.CreateEmpty() .AddJob(Job.Dry.WithPlatform(platform)) - .AddLogger(logger); // make sure we get an output in the TestRunner log + .AddLogger(new OutputLogger(Output)); // make sure we get an output in the TestRunner log // CanExecute ensures that at least one benchmark has executed successfully CanExecute(benchmark, config, fullValidation: true); diff --git a/tests/BenchmarkDotNet.IntegrationTests/RoslynToolchainTest.cs b/tests/BenchmarkDotNet.IntegrationTests/RoslynToolchainTest.cs index 472ba3907a..74fcb3137b 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/RoslynToolchainTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/RoslynToolchainTest.cs @@ -31,9 +31,8 @@ public void CanExecuteWithNonDefaultUiCulture(string culture) CultureInfo.CurrentCulture = overrideCulture; CultureInfo.CurrentUICulture = overrideCulture; - var logger = new OutputLogger(Output); var miniJob = Job.Dry.WithToolchain(RoslynToolchain.Instance); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); CanExecute(config); } diff --git a/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs b/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs index 82afc50466..a915c2d60b 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/SetupAndCleanupTests.cs @@ -78,9 +78,8 @@ public SetupAndCleanupTests(ITestOutputHelper output) : base(output) { } [Fact] public void AllSetupAndCleanupMethodRunsForSpecificBenchmark() { - var logger = new OutputLogger(Output); var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(logger, miniJob); + var config = CreateSimpleConfig(job: miniJob); var summary = CanExecute(config); var standardOutput = GetCombinedStandardOutput(summary);