Skip to content

Commit

Permalink
Apply rework from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Tim Cassell <[email protected]>
  • Loading branch information
leonvandermeer and timcassell committed Nov 29, 2024
1 parent 6099ed0 commit 242589b
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 27 deletions.
2 changes: 1 addition & 1 deletion docs/articles/guides/console-args.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ dotnet run -c Release -- --filter * --runtimes net6.0 net8.0 --statisticalTest 5
* `--platform` the Platform that should be used. If not specified, the host process platform is used (default). AnyCpu/X86/X64/Arm/Arm64/LoongArch64.
* `--runOncePerIteration` run the benchmark exactly once per iteration.
* `--buildTimeout` build timeout in seconds.
* `--preventSleep` prevents the system from entering sleep or turning off the display. No/RequireSystem/RequireDisplay.
* `--preventSleep` prevents the system from entering sleep or turning off the display. None/RequireSystem/RequireDisplay.
* `--wasmEngine` full path to a java script engine used to run the benchmarks, used by Wasm toolchain.
* `--wasmMainJS` path to the test-main.js file used by Wasm toolchain. Mandatory when using \"--runtimes wasm\"
* `--expose_wasm` arguments for the JavaScript engine used by Wasm toolchain.
Expand Down
4 changes: 2 additions & 2 deletions docs/articles/samples/IntroWakeLock.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ uid: BenchmarkDotNet.Samples.IntroWakeLock

## Sample: IntroWakeLock

Running Benchmarks usually takes enough time such that the system enters sleep or turns of the display.
Running benchmarks may sometimes take enough time such that the system enters sleep or turns off the display.

Using a WakeLock prevents the system doing so.

Expand All @@ -15,7 +15,7 @@ Using a WakeLock prevents the system doing so.
### Command line

```
--preventSleep No
--preventSleep None
```
```
--preventSleep RequireSystem
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Configs/DefaultConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public IEnumerable<IValidator> GetValidators()

public TimeSpan BuildTimeout => TimeSpan.FromSeconds(120);

public WakeLockType WakeLock => WakeLockType.No;
public WakeLockType WakeLock => WakeLockType.RequireSystem;

public string ArtifactsPath
{
Expand Down
8 changes: 4 additions & 4 deletions src/BenchmarkDotNet/Configs/WakeLockType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
public enum WakeLockType
{
/// <summary>
/// Allows the system to enter sleep and/or turn off the display while benchmarks are running.
/// Forces the system to be in the working state while benchmarks are running.
/// </summary>
No,
RequireSystem,

/// <summary>
/// Forces the system to be in the working state while benchmarks are running.
/// Allows the system to enter sleep and/or turn off the display while benchmarks are running.
/// </summary>
RequireSystem,
None,

/// <summary>
/// Forces the display to be on while benchmarks are running.
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public bool UseDisassemblyDiagnoser
[Option("buildTimeout", Required = false, HelpText = "Build timeout in seconds.")]
public int? TimeOutInSeconds { get; set; }

[Option("preventSleep", Required = false, HelpText = "Prevents the system from entering sleep or turning off the display. No/RequireSystem/RequireDisplay.")]
[Option("preventSleep", Required = false, HelpText = "Prevents the system from entering sleep or turning off the display. None/RequireSystem/RequireDisplay.")]
public WakeLockType? WakeLock { get; set; }

[Option("stopOnFirstError", Required = false, Default = false, HelpText = "Stop on first error.")]
Expand Down
6 changes: 5 additions & 1 deletion src/BenchmarkDotNet/Running/WakeLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@ internal partial class WakeLock
public static WakeLockType GetWakeLockType(BenchmarkRunInfo[] benchmarkRunInfos) =>
benchmarkRunInfos.Select(static i => i.Config.WakeLock).Max();

private static readonly bool OsVersionIsSupported =
// Must be windows 7 or greater
OsDetector.IsWindows() && Environment.OSVersion.Version >= new Version(6, 1);

public static IDisposable Request(WakeLockType wakeLockType, string reason) =>
wakeLockType == WakeLockType.No || !OsDetector.IsWindows() ? null : new WakeLockSentinel(wakeLockType, reason);
wakeLockType == WakeLockType.None || !OsVersionIsSupported ? null : new WakeLockSentinel(wakeLockType, reason);

private class WakeLockSentinel : DisposeAtProcessTermination
{
Expand Down
17 changes: 10 additions & 7 deletions tests/BenchmarkDotNet.IntegrationTests/WakeLockTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ public class WakeLockTests(ITestOutputHelper output) : BenchmarkTestExecutor(out
[Fact]
public void ConfigurationDefaultValue()
{
Assert.Equal(WakeLockType.No, DefaultConfig.Instance.WakeLock);
Assert.Equal(WakeLockType.No, new DebugBuildConfig().WakeLock);
Assert.Equal(WakeLockType.No, new DebugInProcessConfig().WakeLock);
Assert.Equal(WakeLockType.RequireSystem, DefaultConfig.Instance.WakeLock);
Assert.Equal(WakeLockType.RequireSystem, new DebugBuildConfig().WakeLock);
Assert.Equal(WakeLockType.RequireSystem, new DebugInProcessConfig().WakeLock);
}

[TheoryEnvSpecific(EnvRequirement.NonWindows)]
[InlineData(WakeLockType.No)]
[InlineData(WakeLockType.None)]
[InlineData(WakeLockType.RequireSystem)]
[InlineData(WakeLockType.RequireDisplay)]
public void WakeLockIsWindowsOnly(WakeLockType wakeLockType)
Expand All @@ -43,7 +43,7 @@ public void WakeLockIsWindowsOnly(WakeLockType wakeLockType)
[FactEnvSpecific(EnvRequirement.WindowsOnly)]
public void WakeLockSleepOrDisplayIsAllowed()
{
using IDisposable wakeLock = WakeLock.Request(WakeLockType.No, "dummy");
using IDisposable wakeLock = WakeLock.Request(WakeLockType.None, "dummy");
Assert.Null(wakeLock);
}

Expand Down Expand Up @@ -83,7 +83,8 @@ [Benchmark] public void Sleep() { }
[SupportedOSPlatform("windows")]
#endif
[TheoryEnvSpecific(EnvRequirement.WindowsOnly)]
[InlineData(typeof(Sleepy), "")]
[InlineData(typeof(Default), "SYSTEM")]
[InlineData(typeof(None), "")]
[InlineData(typeof(RequireSystem), "SYSTEM")]
[InlineData(typeof(RequireDisplay), "DISPLAY, SYSTEM")]
public async Task BenchmarkRunnerAcquiresWakeLock(Type type, string expected)
Expand All @@ -106,7 +107,9 @@ async Task WaitForBenchmarkRunningAndGetPowerRequests()
}
}

public class Sleepy : Base { }
public class Default : Base { }

[WakeLock(WakeLockType.None)] public class None : Base { }

[WakeLock(WakeLockType.RequireSystem)] public class RequireSystem : Base { }

Expand Down
6 changes: 3 additions & 3 deletions tests/BenchmarkDotNet.Tests/ConfigParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -367,17 +367,17 @@ public void WhenUserDoesNotSpecifyTimeoutTheDefaultValueIsUsed()
[Fact]
public void UserCanSpecifyWakeLock()
{
var config = ConfigParser.Parse(["--preventSleep", "RequireSystem"], new OutputLogger(Output)).config;
var config = ConfigParser.Parse(["--preventSleep", "RequireDisplay"], new OutputLogger(Output)).config;

Assert.Equal(WakeLockType.RequireSystem, config.WakeLock);
Assert.Equal(WakeLockType.RequireDisplay, config.WakeLock);
}

[Fact]
public void WhenUserDoesNotSpecifyWakeLockTheDefaultValueIsUsed()
{
var config = ConfigParser.Parse([], new OutputLogger(Output)).config;

Assert.Equal(WakeLockType.No, config.WakeLock);
Assert.Equal(DefaultConfig.Instance.WakeLock, config.WakeLock);
}

[Theory]
Expand Down
14 changes: 7 additions & 7 deletions tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -387,26 +387,26 @@ public void WhenWakeLockIsNotSpecifiedTheDefaultValueIsUsed()
[Fact]
public void CustomWakeLockHasPrecedenceOverDefaultWakeLock()
{
WakeLockType customTimeout = WakeLockType.RequireDisplay;
var mutable = ManualConfig.CreateEmpty().WithWakeLock(customTimeout);
WakeLockType customWakeLock = WakeLockType.RequireDisplay;
var mutable = ManualConfig.CreateEmpty().WithWakeLock(customWakeLock);
var final = ImmutableConfigBuilder.Create(mutable);
Assert.Equal(customTimeout, final.WakeLock);
Assert.Equal(customWakeLock, final.WakeLock);
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void WhenTwoCustomWakeLocksAreProvidedTheLongerOneIsUsed(bool direction)
{
var system = ManualConfig.CreateEmpty().WithWakeLock(WakeLockType.RequireSystem);
var none = ManualConfig.CreateEmpty().WithWakeLock(WakeLockType.None);
var display = ManualConfig.CreateEmpty().WithWakeLock(WakeLockType.RequireDisplay);

if (direction)
system.Add(display);
none.Add(display);
else
display.Add(system);
display.Add(none);

var final = ImmutableConfigBuilder.Create(direction ? system : display);
var final = ImmutableConfigBuilder.Create(direction ? none : display);
Assert.Equal(WakeLockType.RequireDisplay, final.WakeLock);
}

Expand Down

0 comments on commit 242589b

Please sign in to comment.