Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
Expand All @@ -17,7 +17,6 @@
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Loggers;
using BenchmarkDotNet.Mathematics;
using BenchmarkDotNet.Portability;
using BenchmarkDotNet.Reports;
using BenchmarkDotNet.Toolchains;
using BenchmarkDotNet.Toolchains.Parameters;
Expand Down Expand Up @@ -168,14 +167,14 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
var cultureInfo = config.CultureInfo ?? DefaultCultureInfo.Instance;
var reports = new List<BenchmarkReport>();
string title = GetTitle(new[] { benchmarkRunInfo });
var consoleTitle = RuntimeInformation.IsWindows() ? Console.Title : string.Empty;
using var consoleTitler = new ConsoleTitler("");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- using var consoleTitler = new ConsoleTitler("");
+ using var consoleTitler = new ConsoleTitler($"{benchmarksToRunCount}/{totalBenchmarkCount} Remaining");


logger.WriteLineInfo($"// Found {benchmarks.Length} benchmarks:");
foreach (var benchmark in benchmarks)
logger.WriteLineInfo($"// {benchmark.DisplayInfo}");
logger.WriteLine();

UpdateTitle(totalBenchmarkCount, benchmarksToRunCount);
UpdateTitle(totalBenchmarkCount, benchmarksToRunCount, consoleTitler);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this update title line since it's passed in the constructor.


using (var powerManagementApplier = new PowerManagementApplier(logger))
{
Expand Down Expand Up @@ -241,15 +240,10 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,

benchmarksToRunCount -= stop ? benchmarks.Length - i : 1;

LogProgress(logger, in runsChronometer, totalBenchmarkCount, benchmarksToRunCount, taskbarProgress);
LogProgress(logger, in runsChronometer, totalBenchmarkCount, benchmarksToRunCount, consoleTitler, taskbarProgress);
}
}

if (RuntimeInformation.IsWindows())
{
Console.Title = consoleTitle;
}

var runEnd = runsChronometer.GetElapsed();

return new Summary(title,
Expand Down Expand Up @@ -652,15 +646,12 @@ private static void Cleanup(HashSet<string> artifactsToCleanup)
}
}

private static void UpdateTitle(int totalBenchmarkCount, int benchmarksToRunCount)
private static void UpdateTitle(int totalBenchmarkCount, int benchmarksToRunCount, ConsoleTitler consoleTitler)
{
if (!Console.IsOutputRedirected && (RuntimeInformation.IsWindows() || RuntimeInformation.IsLinux() || RuntimeInformation.IsMacOS()))
{
Console.Title = $"{benchmarksToRunCount}/{totalBenchmarkCount} Remaining";
}
consoleTitler.UpdateTitle(() => $"{benchmarksToRunCount}/{totalBenchmarkCount} Remaining");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateTitle function is no longer needed, just call consoleTitler.UpdateTitle directly.


private static void LogProgress(ILogger logger, in StartedClock runsChronometer, int totalBenchmarkCount, int benchmarksToRunCount, TaskbarProgress taskbarProgress)
private static void LogProgress(ILogger logger, in StartedClock runsChronometer, int totalBenchmarkCount, int benchmarksToRunCount, ConsoleTitler consoleTitler, TaskbarProgress taskbarProgress)
{
int executedBenchmarkCount = totalBenchmarkCount - benchmarksToRunCount;
TimeSpan fromNow = GetEstimatedFinishTime(runsChronometer, benchmarksToRunCount, executedBenchmarkCount);
Expand All @@ -669,10 +660,8 @@ private static void LogProgress(ILogger logger, in StartedClock runsChronometer,
$" Estimated finish {estimatedEnd:yyyy-MM-dd H:mm} ({(int)fromNow.TotalHours}h {fromNow.Minutes}m from now) **";
logger.WriteLineHeader(message);

if (!Console.IsOutputRedirected && (RuntimeInformation.IsWindows() || RuntimeInformation.IsLinux() || RuntimeInformation.IsMacOS()))
{
Console.Title = $"{benchmarksToRunCount}/{totalBenchmarkCount} Remaining - {(int)fromNow.TotalHours}h {fromNow.Minutes}m to finish";
}
consoleTitler.UpdateTitle (() => $"{benchmarksToRunCount}/{totalBenchmarkCount} Remaining - {(int)fromNow.TotalHours}h {fromNow.Minutes}m to finish");

taskbarProgress.SetProgress((float) executedBenchmarkCount / totalBenchmarkCount);
}

Expand Down
87 changes: 87 additions & 0 deletions src/BenchmarkDotNet/Running/ConsoleTitler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
using System;
using System.IO;
using BenchmarkDotNet.Portability;

namespace BenchmarkDotNet.Running
{
/// <summary>
/// Updates Console.Title, subject to platform capabilities and Console availability.
/// Restores the original (or fallback) title upon disposal.
/// </summary>
internal class ConsoleTitler : IDisposable
{
/// <summary>
/// Whether this instance has any effect. This will be false if the platform doesn't support Console retitling,
/// or if Console output is redirected.
/// </summary>
public bool IsEnabled { get; private set; }

private string oldConsoleTitle;

/// <summary>
/// Constructs a ConsoleTitler
/// </summary>
/// <param name="fallbackTitle">What to restore Console.Title to upon disposal (for platforms with write-only Console.Title)</param>
public ConsoleTitler(string fallbackTitle)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the passed in string would be the initial title to write, not the fallback title on dispose. I don't think there is any value in having a custom fallback title on dispose.

{
// Return without enabling if Console output is redirected, or if we're not on a platform that supports Console retitling.
if (Console.IsOutputRedirected || !PlatformSupportsTitleWrite())
{
return;
}

try
{
oldConsoleTitle = PlatformSupportsTitleRead() ? Console.Title : fallbackTitle;
IsEnabled = true;
}
catch (IOException)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be simplified to

try
{
    oldConsoleTitle = Console.Title
}
catch
{
    oldConsoleTitle = string.Empty;
}

That should make it more future-proof.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require a #pragma to suppress CA1416 - you OK with that?

Console.Title's get accessor is defined with [SupportedOSPlatform("windows")]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I suppose it makes sense to have the platform check for the get since that's currently the only supported platform. It can be updated in the future if that ever changes. But no platform check for the set. I still think the simpler try/catch makes sense, though, even with the platform check.

{
// We're unable to read Console.Title on a platform that supports it. This can happen when no console
// window is available due to the application being Windows Forms, WPF, Windows Service or a daemon.
// Because we won't be able to write Console.Title either, return without enabling the titler.
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write the passed in title here. This is where you need the flag to check in UpdateTitle if it is writable. The flag you have now only checks if it's readable.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need a separate try/catch for the write instead of a platform check. We'd want to catch IOException and PlatformNotSupportedException. Let other exceptions propagate.

try
{
   Console.Title = title;
   IsEnabled = true;
}
catch (IOException) { }
catch (PlatformNotSupportedException) { }

}

#if NET6_0_OR_GREATER
[System.Runtime.Versioning.SupportedOSPlatformGuard("windows")]
#endif
private static bool PlatformSupportsTitleRead() => RuntimeInformation.IsWindows();

private static bool PlatformSupportsTitleWrite() =>
RuntimeInformation.IsWindows() ||
RuntimeInformation.IsLinux() ||
RuntimeInformation.IsMacOS();

/// <summary>
/// Updates Console.Title if enabled.
/// </summary>
public void UpdateTitle(string title)
{
if (IsEnabled)
{
Console.Title = title;
}
}

/// <summary>
/// Updates Console.Title if enabled, using a Func to avoid potential string-building cost.
/// </summary>
public void UpdateTitle(Func<string> title)
{
if (IsEnabled)
{
Console.Title = title();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary. The cost of string building in this case is minimal, and this is just adding the cost of a delegate and closures.


public void Dispose()
{
if (IsEnabled && oldConsoleTitle != null)
{
Console.Title = oldConsoleTitle;
IsEnabled = false;
}
}
}
}