Extract TreeRebuildCoalescer from HardwareMonitorService
Moves the hardware-change debounce/coalescing worker (the dirty/queued flags and re-queue logic) into a dedicated class with an injectable delay, so the concurrency behavior can be unit-tested deterministically instead of living as an untestable fire-and-forget block in the service. Behavior is unchanged. Adds 3 tests (single rebuild after delay, burst coalesced to one, no rebuild when closed) driven by a controllable delay gate. 199 tests pass (was 196). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0.
|
||||
// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
// Copyright (C) LibreHardwareMonitor and Contributors.
|
||||
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using LibreHardwareMonitor.Windows.WinUI.Services;
|
||||
using Xunit;
|
||||
|
||||
namespace LibreHardwareMonitor.Windows.WinUI.Tests.Services;
|
||||
|
||||
public class TreeRebuildCoalescerTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task Request_RebuildsOnceAfterDelay()
|
||||
{
|
||||
var gate = new TaskCompletionSource();
|
||||
int rebuilds = 0;
|
||||
var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => true, () => gate.Task);
|
||||
|
||||
coalescer.Request();
|
||||
Assert.Equal(0, rebuilds); // worker is blocked on the (not-yet-completed) delay
|
||||
|
||||
gate.SetResult();
|
||||
await coalescer.RebuildLoopTask;
|
||||
|
||||
Assert.Equal(1, rebuilds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Request_CoalescesBurstIntoSingleRebuild()
|
||||
{
|
||||
var gate = new TaskCompletionSource();
|
||||
int rebuilds = 0;
|
||||
var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => true, () => gate.Task);
|
||||
|
||||
coalescer.Request();
|
||||
coalescer.Request();
|
||||
coalescer.Request();
|
||||
|
||||
gate.SetResult();
|
||||
await coalescer.RebuildLoopTask;
|
||||
|
||||
Assert.Equal(1, rebuilds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Request_DoesNotRebuildWhenCannotRebuild()
|
||||
{
|
||||
var gate = new TaskCompletionSource();
|
||||
int rebuilds = 0;
|
||||
var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => false, () => gate.Task);
|
||||
|
||||
coalescer.Request();
|
||||
gate.SetResult();
|
||||
await coalescer.RebuildLoopTask;
|
||||
|
||||
Assert.Equal(0, rebuilds);
|
||||
}
|
||||
}
|
||||
@@ -3,7 +3,6 @@
|
||||
// Copyright (C) LibreHardwareMonitor and Contributors.
|
||||
|
||||
using System;
|
||||
using System.Diagnostics;
|
||||
using System.Linq;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
@@ -27,13 +26,13 @@ public sealed class HardwareMonitorService : IDisposable
|
||||
|
||||
private readonly object _updateLock = new();
|
||||
private readonly UpdateVisitor _updateVisitor = new();
|
||||
private readonly TreeRebuildCoalescer _treeRebuildCoalescer;
|
||||
private bool _isOpen;
|
||||
private int _treeRebuildQueued;
|
||||
private volatile bool _treeRebuildDirty;
|
||||
|
||||
public HardwareMonitorService(AppSettings settings)
|
||||
{
|
||||
Settings = settings;
|
||||
_treeRebuildCoalescer = new TreeRebuildCoalescer(() => RebuildTree(), () => _isOpen);
|
||||
ApplyWinUiHardwareDefaults();
|
||||
Computer = new Computer(settings);
|
||||
Computer.HardwareAdded += HardwareChanged;
|
||||
@@ -269,42 +268,7 @@ public sealed class HardwareMonitorService : IDisposable
|
||||
storageDevice.ForceWakeup = Settings.GetValue("forceDriveWakeupItem", false);
|
||||
|
||||
if (_isOpen)
|
||||
QueueTreeRebuild();
|
||||
_treeRebuildCoalescer.Request();
|
||||
}
|
||||
|
||||
private void QueueTreeRebuild()
|
||||
{
|
||||
_treeRebuildDirty = true;
|
||||
if (Interlocked.Exchange(ref _treeRebuildQueued, 1) == 1)
|
||||
return;
|
||||
|
||||
_ = Task.Run(async () =>
|
||||
{
|
||||
try
|
||||
{
|
||||
await Task.Delay(100).ConfigureAwait(false);
|
||||
|
||||
// Rebuild until no change has arrived since the last rebuild began, so a HardwareAdded/Removed that lands
|
||||
// while a rebuild is in flight is not coalesced away and lost.
|
||||
while (_treeRebuildDirty)
|
||||
{
|
||||
_treeRebuildDirty = false;
|
||||
if (_isOpen)
|
||||
RebuildTree();
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Debug.WriteLine($"Hardware tree rebuild failed: {ex}");
|
||||
}
|
||||
finally
|
||||
{
|
||||
Interlocked.Exchange(ref _treeRebuildQueued, 0);
|
||||
|
||||
// A change may have slipped in between the loop's exit and clearing the flag; re-queue so it is honored.
|
||||
if (_treeRebuildDirty)
|
||||
QueueTreeRebuild();
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0.
|
||||
// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
// Copyright (C) LibreHardwareMonitor and Contributors.
|
||||
|
||||
using System;
|
||||
using System.Diagnostics;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
namespace LibreHardwareMonitor.Windows.WinUI.Services;
|
||||
|
||||
/// <summary>
|
||||
/// Coalesces a burst of hardware-change notifications into as few tree rebuilds as possible without dropping the last
|
||||
/// one. After the first request a single worker waits a short debounce delay, then rebuilds while more changes keep
|
||||
/// arriving; a change that lands between the worker finishing and the queued flag clearing re-arms the worker.
|
||||
/// Extracted from <see cref="HardwareMonitorService" /> so this concurrency logic can be unit-tested with a controllable
|
||||
/// delay.
|
||||
/// </summary>
|
||||
internal sealed class TreeRebuildCoalescer
|
||||
{
|
||||
private readonly Action _rebuild;
|
||||
private readonly Func<bool> _canRebuild;
|
||||
private readonly Func<Task> _delay;
|
||||
private int _queued;
|
||||
private volatile bool _dirty;
|
||||
|
||||
public TreeRebuildCoalescer(Action rebuild, Func<bool> canRebuild, Func<Task>? delay = null)
|
||||
{
|
||||
_rebuild = rebuild;
|
||||
_canRebuild = canRebuild;
|
||||
_delay = delay ?? (() => Task.Delay(100));
|
||||
}
|
||||
|
||||
/// <summary>The most recently started worker. Exposed only so tests can deterministically await a rebuild cycle.</summary>
|
||||
internal Task RebuildLoopTask { get; private set; } = Task.CompletedTask;
|
||||
|
||||
public void Request()
|
||||
{
|
||||
_dirty = true;
|
||||
if (Interlocked.Exchange(ref _queued, 1) == 1)
|
||||
return;
|
||||
|
||||
RebuildLoopTask = Task.Run(async () =>
|
||||
{
|
||||
try
|
||||
{
|
||||
await _delay().ConfigureAwait(false);
|
||||
|
||||
// Rebuild until no change has arrived since the last rebuild began, so a change that lands while a
|
||||
// rebuild is in flight is not coalesced away and lost.
|
||||
while (_dirty)
|
||||
{
|
||||
_dirty = false;
|
||||
if (_canRebuild())
|
||||
_rebuild();
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Debug.WriteLine($"Hardware tree rebuild failed: {ex}");
|
||||
}
|
||||
finally
|
||||
{
|
||||
Interlocked.Exchange(ref _queued, 0);
|
||||
|
||||
// A change may have slipped in between the loop's exit and clearing the flag; re-queue so it is honored.
|
||||
if (_dirty)
|
||||
Request();
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user