Skip to content

Commit 7eb08b1

Browse files
author
Moritz Scherer
committed
[circt-lsp-server] Make time source injectable
Introduce an injectable clock and test-only wait shim to remove real sleeps from `PendingChanges` logic in tests, while preserving production behavior. - Add an **injectable time source**: - `using SteadyClock = std::chrono::steady_clock;` - `using NowFn = std::function<SteadyClock::time_point()>;` - Store `NowFn nowFn` and a `bool useManualClock` in `PendingChangesMap`. - Replace direct calls to `std::chrono::steady_clock::now()` with `nowFn()`. - Add `waitForMinMs(uint64_t ms, SteadyClock::time_point start)`: - **Prod path**: `std::this_thread::sleep_for(ms)`. - **Test path** (`useManualClock`): busy-wait with `std::this_thread::yield()` until `nowFn()` reaches target. - New ctor overload for tests: - `PendingChangesMap(unsigned maxThreads, NowFn now)` -> enables manual clock mode. - Dtor now calls `abort()` to ensure all pending work is cleared on teardown. - Replace sleep in debounce worker: - `sleep_for(ms)` -> `waitForMinMs(ms, scheduleTime)`. - Add `ManualClock` with manual advancement (`advanceMs`) and `now()` accessor. - Utility `advanceTestTime(clock, ms)` that advances test time and yields. - `CallbackCapture::waitFor()` now uses a tight fixed 100ms window (no per-call timeout arg). - Refactor tests to: - Construct `PendingChangesMap` with manual clock (`PendingChangesMap(2, [&]{ return clock.now(); })`). - Advance time deterministically instead of sleeping. - Validate debounce min/obsolete/max-cap behaviors without flakiness. - Adjust debounce cap in `MaxCapForcesFlushDuringContinuousTyping` to 1ms to tighten assertions. - Sprinkle end-of-test large time advances to flush any queued workers.
1 parent c429d8f commit 7eb08b1

File tree

3 files changed

+133
-44
lines changed

3 files changed

+133
-44
lines changed

lib/Tools/circt-verilog-lsp-server/Utils/PendingChanges.cpp

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,7 @@ void PendingChangesMap::debounceAndUpdate(
4848

4949
void PendingChangesMap::enqueueChange(
5050
const llvm::lsp::DidChangeTextDocumentParams &params) {
51-
// Key by normalized LSP file path. If your pipeline allows multiple
52-
// spellings (symlinks/case), normalize upstream or canonicalize here.
53-
const auto now = std::chrono::steady_clock::now();
51+
const auto now = nowFn();
5452
const std::string key = params.textDocument.uri.file().str();
5553

5654
std::scoped_lock lock(mu);
@@ -71,7 +69,7 @@ void PendingChangesMap::debounceAndThen(
7169
DebounceOptions options,
7270
std::function<void(std::unique_ptr<PendingChanges>)> cb) {
7371
const std::string key = params.textDocument.uri.file().str();
74-
const auto scheduleTime = std::chrono::steady_clock::now();
72+
const auto scheduleTime = nowFn();
7573

7674
// If debounce is disabled, run on main thread
7775
if (options.disableDebounce) {
@@ -87,8 +85,7 @@ void PendingChangesMap::debounceAndThen(
8785
// Simple timer: sleep min-quiet before checking. We rely on the fact
8886
// that newer edits can arrive while we sleep, updating lastChangeTime.
8987
if (options.debounceMinMs > 0)
90-
std::this_thread::sleep_for(
91-
std::chrono::milliseconds(options.debounceMinMs));
88+
waitForMinMs(options.debounceMinMs, scheduleTime);
9289

9390
std::unique_ptr<PendingChanges>
9491
result; // decided under lock, callback after
@@ -98,7 +95,7 @@ void PendingChangesMap::debounceAndThen(
9895
auto it = pending.find(key);
9996
if (it != pending.end()) {
10097
PendingChanges &pc = it->second;
101-
const auto now = std::chrono::steady_clock::now();
98+
const auto now = nowFn();
10299

103100
// quietSinceSchedule: if no newer edits arrived after we scheduled
104101
// this task, then we consider the burst "quiet" and flush now.
@@ -142,5 +139,20 @@ PendingChangesMap::takeAndErase(llvm::StringMap<PendingChanges>::iterator it) {
142139
return out;
143140
}
144141

142+
void PendingChangesMap::waitForMinMs(uint64_t ms,
143+
SteadyClock::time_point start) {
144+
if (!ms)
145+
return;
146+
if (!useManualClock) {
147+
std::this_thread::sleep_for(std::chrono::milliseconds(ms));
148+
return;
149+
}
150+
// Manual clock: busy-wait with yields until now() reaches start + ms.
151+
const auto target = start + std::chrono::milliseconds(ms);
152+
while (nowFn() < target) {
153+
std::this_thread::yield();
154+
}
155+
}
156+
145157
} // namespace lsp
146158
} // namespace circt

lib/Tools/circt-verilog-lsp-server/Utils/PendingChanges.h

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@
1717

1818
#include <chrono>
1919
#include <cstdint>
20+
#include <functional>
2021
#include <mutex>
2122
#include <thread>
2223
#include <vector>
2324

2425
namespace circt {
2526
namespace lsp {
2627

28+
using SteadyClock = std::chrono::steady_clock;
29+
using NowFn = std::function<SteadyClock::time_point()>;
30+
2731
/// Build a pool strategy with a sensible minimum.
2832
static llvm::ThreadPoolStrategy makeStrategy(unsigned maxThreads) {
2933
llvm::ThreadPoolStrategy s = llvm::hardware_concurrency();
@@ -56,7 +60,16 @@ class PendingChangesMap {
5660
public:
5761
explicit PendingChangesMap(
5862
unsigned maxThreads = std::thread::hardware_concurrency())
59-
: pool(makeStrategy(maxThreads)), tasks(pool) {}
63+
: pool(makeStrategy(maxThreads)), tasks(pool),
64+
nowFn([] { return SteadyClock::now(); }), useManualClock(false) {}
65+
66+
// Test-only path: provide a manual clock source.
67+
explicit PendingChangesMap(unsigned maxThreads, NowFn now)
68+
: pool(makeStrategy(maxThreads)), tasks(pool), nowFn(std::move(now)),
69+
useManualClock(true) {}
70+
71+
/// Destructor ensures all pending work is cleared.
72+
~PendingChangesMap() { abort(); }
6073

6174
/// Call during server shutdown; Erase all file changes, then clear file map.
6275
/// Thread-safe.
@@ -106,6 +119,13 @@ class PendingChangesMap {
106119
/// Internal concurrency used for sleeps + checks.
107120
llvm::StdThreadPool pool;
108121
llvm::ThreadPoolTaskGroup tasks;
122+
123+
/// Injectable time source for unit testing
124+
NowFn nowFn;
125+
bool useManualClock;
126+
127+
/// Test wrapper around a thread timeout
128+
void waitForMinMs(uint64_t ms, SteadyClock::time_point start);
109129
};
110130

111131
} // namespace lsp

0 commit comments

Comments
 (0)