Skip to content

Commit 8add0b4

Browse files
committed
Refactor session concurrency: replace ReaderWriterLockSlim with lock+snapshot; add BDN microbench (lock vs ConcurrentQueue); preserve early-out CaptureSqlText=false.
1 parent 4daf457 commit 8add0b4

File tree

5 files changed

+164
-51
lines changed

5 files changed

+164
-51
lines changed

KeelMatrix.QueryWatch.sln

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "bench", "bench", "{760AB15A
2929
EndProject
3030
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "KeelMatrix.QueryWatch.Redaction.Benchmarks", "bench\KeelMatrix.QueryWatch.Redaction.Benchmarks\KeelMatrix.QueryWatch.Redaction.Benchmarks.csproj", "{FE18583F-2A73-43C3-9EAF-94CA8292F5B8}"
3131
EndProject
32+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "KeelMatrix.QueryWatch.Benchmarks", "bench\KeelMatrix.QueryWatch.Benchmarks\KeelMatrix.QueryWatch.Benchmarks.csproj", "{F483136B-5F73-44F1-8BA3-718B200B258A}"
33+
EndProject
3234
Global
3335
GlobalSection(SolutionConfigurationPlatforms) = preSolution
3436
Debug|Any CPU = Debug|Any CPU
@@ -147,6 +149,18 @@ Global
147149
{FE18583F-2A73-43C3-9EAF-94CA8292F5B8}.Release|x64.Build.0 = Release|Any CPU
148150
{FE18583F-2A73-43C3-9EAF-94CA8292F5B8}.Release|x86.ActiveCfg = Release|Any CPU
149151
{FE18583F-2A73-43C3-9EAF-94CA8292F5B8}.Release|x86.Build.0 = Release|Any CPU
152+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
153+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|Any CPU.Build.0 = Debug|Any CPU
154+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|x64.ActiveCfg = Debug|Any CPU
155+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|x64.Build.0 = Debug|Any CPU
156+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|x86.ActiveCfg = Debug|Any CPU
157+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Debug|x86.Build.0 = Debug|Any CPU
158+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|Any CPU.ActiveCfg = Release|Any CPU
159+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|Any CPU.Build.0 = Release|Any CPU
160+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|x64.ActiveCfg = Release|Any CPU
161+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|x64.Build.0 = Release|Any CPU
162+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|x86.ActiveCfg = Release|Any CPU
163+
{F483136B-5F73-44F1-8BA3-718B200B258A}.Release|x86.Build.0 = Release|Any CPU
150164
EndGlobalSection
151165
GlobalSection(SolutionProperties) = preSolution
152166
HideSolutionNode = FALSE
@@ -161,6 +175,7 @@ Global
161175
{D85E1811-E38E-461B-B817-D445B9F4B11F} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
162176
{5BE6CC64-5276-4832-ADB1-AD0D334D26D1} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
163177
{FE18583F-2A73-43C3-9EAF-94CA8292F5B8} = {760AB15A-8938-8D9B-BEDE-5CE1484B84C3}
178+
{F483136B-5F73-44F1-8BA3-718B200B258A} = {760AB15A-8938-8D9B-BEDE-5CE1484B84C3}
164179
EndGlobalSection
165180
GlobalSection(ExtensibilityGlobals) = postSolution
166181
SolutionGuid = {B433EB21-BD3B-4E85-9645-38C1E8BDC563}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<TargetFramework>net8.0</TargetFramework>
5+
<IsPackable>false</IsPackable>
6+
<Nullable>enable</Nullable>
7+
<ImplicitUsings>enable</ImplicitUsings>
8+
<TreatWarningsAsErrors>false</TreatWarningsAsErrors>
9+
<GenerateDocumentationFile>false</GenerateDocumentationFile>
10+
</PropertyGroup>
11+
12+
<ItemGroup>
13+
<PackageReference Include="BenchmarkDotNet" />
14+
</ItemGroup>
15+
16+
<ItemGroup>
17+
<ProjectReference Include="../../src/KeelMatrix.QueryWatch/KeelMatrix.QueryWatch.csproj" />
18+
</ItemGroup>
19+
</Project>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
using BenchmarkDotNet.Running;
2+
3+
BenchmarkRunner.Run<KeelMatrix.QueryWatch.Benchmarks.RecordThroughputBench>();
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Microbenchmarks for write-path contention in QueryWatchSession.
2+
// Compares production "lock + copy-on-stop" vs an alternative "ConcurrentQueue + snapshot".
3+
#nullable enable
4+
using System.Collections.Concurrent;
5+
using BenchmarkDotNet.Attributes;
6+
7+
namespace KeelMatrix.QueryWatch.Benchmarks {
8+
9+
[MemoryDiagnoser]
10+
#if DEBUG
11+
[SimpleJob(warmupCount: 1, iterationCount: 2)]
12+
#else
13+
[SimpleJob] // default job in Release
14+
#endif
15+
public class RecordThroughputBench {
16+
[Params(1, 2, 4, 8, 16)]
17+
public int Threads { get; set; } = 4;
18+
19+
[Params(2_000)]
20+
public int EventsPerThread { get; set; } = 2_000;
21+
22+
private KeelMatrix.QueryWatch.QueryWatchOptions _optsNoText = null!;
23+
24+
[GlobalSetup]
25+
public void Setup() {
26+
_optsNoText = new KeelMatrix.QueryWatch.QueryWatchOptions {
27+
CaptureSqlText = false // exercise the hot path without redaction overhead
28+
};
29+
}
30+
31+
[Benchmark(Baseline = true, Description = "lock[List] + copy-on-stop (production)")]
32+
public KeelMatrix.QueryWatch.QueryWatchReport LockList() {
33+
var session = new KeelMatrix.QueryWatch.QueryWatchSession(_optsNoText);
34+
RunWorkers(Threads, EventsPerThread, () => session.Record("SELECT 1", TimeSpan.FromMilliseconds(1)));
35+
return session.Stop();
36+
}
37+
38+
[Benchmark(Description = "ConcurrentQueue + snapshot (alternative)")]
39+
public KeelMatrix.QueryWatch.QueryWatchReport ConcurrentQueueSnapshot() {
40+
var session = new ConcurrentQueueSession(_optsNoText);
41+
RunWorkers(Threads, EventsPerThread, () => session.Record("SELECT 1", TimeSpan.FromMilliseconds(1)));
42+
return session.Stop();
43+
}
44+
45+
private static void RunWorkers(int threads, int eventsPerThread, Action action) {
46+
var tasks = new Task[threads];
47+
for (int t = 0; t < threads; t++) {
48+
tasks[t] = Task.Run(() => {
49+
for (int i = 0; i < eventsPerThread; i++) action();
50+
});
51+
}
52+
Task.WaitAll(tasks);
53+
}
54+
}
55+
56+
// Minimal alternative session used only for benchmarking.
57+
internal sealed class ConcurrentQueueSession {
58+
private readonly ConcurrentQueue<KeelMatrix.QueryWatch.QueryEvent> _q = new();
59+
private readonly KeelMatrix.QueryWatch.QueryWatchOptions _options;
60+
private int _stopped; // 0=running,1=stopped
61+
private readonly DateTimeOffset _startedAt = DateTimeOffset.UtcNow;
62+
private DateTimeOffset? _stoppedAt;
63+
64+
public ConcurrentQueueSession(KeelMatrix.QueryWatch.QueryWatchOptions options) {
65+
_options = options ?? throw new ArgumentNullException(nameof(options));
66+
}
67+
68+
public void Record(string commandText, TimeSpan duration) {
69+
if (Volatile.Read(ref _stopped) != 0)
70+
throw new InvalidOperationException("Session has been stopped; cannot record new events.");
71+
72+
// early-out if CaptureSqlText=false
73+
string text = string.Empty;
74+
if (_options.CaptureSqlText) {
75+
text = commandText ?? string.Empty;
76+
foreach (var r in _options.Redactors) text = r.Redact(text);
77+
}
78+
79+
// second check to minimize recording after stop (not strictly needed for the benchmark)
80+
if (Volatile.Read(ref _stopped) != 0)
81+
throw new InvalidOperationException("Session has been stopped; cannot record new events.");
82+
83+
// FIX: use the public 3-arg ctor; the 4-arg (with meta) is internal
84+
var ev = new KeelMatrix.QueryWatch.QueryEvent(text, duration, DateTimeOffset.UtcNow);
85+
_q.Enqueue(ev);
86+
}
87+
88+
public KeelMatrix.QueryWatch.QueryWatchReport Stop() {
89+
var now = DateTimeOffset.UtcNow;
90+
if (Interlocked.CompareExchange(ref _stopped, 1, 0) == 0) {
91+
_stoppedAt = now;
92+
}
93+
else {
94+
throw new InvalidOperationException("Session has already been stopped.");
95+
}
96+
97+
var arr = _q.ToArray();
98+
var list = new List<KeelMatrix.QueryWatch.QueryEvent>(arr);
99+
return KeelMatrix.QueryWatch.QueryWatchReport.CreateSnapshot(list, _options, _startedAt, _stoppedAt ?? now);
100+
}
101+
}
102+
}
Lines changed: 25 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
1+
// Copyright (c) KeelMatrix
12
#nullable enable
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Threading;
6+
27
namespace KeelMatrix.QueryWatch {
38
/// <summary>
49
/// Collects query events for the lifetime of a session. Thread‑safe for recording.
10+
/// This version uses a simple <c>lock</c> (monitor) for minimal overhead on write‑heavy workloads,
11+
/// and snapshots the list on <see cref="Stop"/>.
512
/// </summary>
613
public sealed class QueryWatchSession : IDisposable {
714
private readonly List<QueryEvent> _events = new List<QueryEvent>();
8-
private readonly ReaderWriterLockSlim _gate = new ReaderWriterLockSlim();
15+
private readonly object _sync = new object();
916
private bool _disposed;
1017
private int _stopped; // 0 = running, 1 = stopped
1118

@@ -18,56 +25,34 @@ public QueryWatchSession(QueryWatchOptions? options = null) {
1825
StartedAt = DateTimeOffset.UtcNow;
1926
}
2027

21-
/// <summary>
22-
/// Options for this session.
23-
/// </summary>
28+
/// <summary>Options for this session.</summary>
2429
public QueryWatchOptions Options { get; }
2530

26-
/// <summary>
27-
/// UTC timestamp when the session started.
28-
/// </summary>
31+
/// <summary>UTC timestamp when the session started.</summary>
2932
public DateTimeOffset StartedAt { get; }
3033

31-
/// <summary>
32-
/// UTC timestamp when the session stopped, or <c>null</c> if still running.
33-
/// </summary>
34+
/// <summary>UTC timestamp when the session stopped, or <c>null</c> if still running.</summary>
3435
public DateTimeOffset? StoppedAt { get; private set; }
3536

36-
/// <summary>
37-
/// Starts a new session.
38-
/// </summary>
39-
/// <param name="options">Optional session options.</param>
40-
/// <returns>The started session.</returns>
37+
/// <summary>Starts a new session.</summary>
4138
public static QueryWatchSession Start(QueryWatchOptions? options = null) => new QueryWatchSession(options);
4239

43-
/// <summary>
44-
/// Records a query execution event.
45-
/// </summary>
46-
/// <param name="commandText">Executed SQL or provider command text.</param>
47-
/// <param name="duration">Execution duration.</param>
48-
public void Record(string commandText, TimeSpan duration) {
49-
Record(commandText, duration, meta: null);
50-
}
40+
/// <summary>Records a query execution event.</summary>
41+
public void Record(string commandText, TimeSpan duration) => Record(commandText, duration, meta: null);
5142

52-
/// <summary>
53-
/// Records a query execution event with optional metadata.
54-
/// </summary>
55-
/// <param name="commandText">Executed SQL or provider command text.</param>
56-
/// <param name="duration">Execution duration.</param>
57-
/// <param name="meta">Optional metadata bag for provider‑specific details.</param>
43+
/// <summary>Records a query execution event with optional metadata.</summary>
5844
public void Record(string commandText, TimeSpan duration, IReadOnlyDictionary<string, object?>? meta) {
5945
if (_disposed) throw new ObjectDisposedException(nameof(QueryWatchSession));
6046

6147
// Fast path: if already stopped, throw as tests expect.
6248
if (Volatile.Read(ref _stopped) != 0)
6349
throw new InvalidOperationException("Session has been stopped; cannot record new events.");
6450

65-
_gate.EnterWriteLock();
66-
try {
51+
lock (_sync) {
6752
if (_stopped != 0)
6853
throw new InvalidOperationException("Session has been stopped; cannot record new events.");
6954

70-
// Optionally redact or drop SQL text.
55+
// Early‑out: if CaptureSqlText=false, avoid any redactor passes and store empty string.
7156
string text = string.Empty;
7257
if (Options.CaptureSqlText) {
7358
text = commandText ?? string.Empty;
@@ -79,15 +64,9 @@ public void Record(string commandText, TimeSpan duration, IReadOnlyDictionary<st
7964
var ev = new QueryEvent(text, duration, DateTimeOffset.UtcNow, meta);
8065
_events.Add(ev);
8166
}
82-
finally {
83-
_gate.ExitWriteLock();
84-
}
8567
}
8668

87-
/// <summary>
88-
/// Stops the session and returns a snapshot report.
89-
/// </summary>
90-
/// <returns>A report representing the recorded data.</returns>
69+
/// <summary>Stops the session and returns a snapshot report.</summary>
9170
public QueryWatchReport Stop() {
9271
if (_disposed) throw new ObjectDisposedException(nameof(QueryWatchSession));
9372

@@ -100,27 +79,22 @@ public QueryWatchReport Stop() {
10079
throw new InvalidOperationException("Session has already been stopped.");
10180
}
10281

103-
// Snapshot under read lock
104-
_gate.EnterReadLock();
105-
try {
106-
return QueryWatchReport.CreateSnapshot(_events, Options, StartedAt, StoppedAt ?? now);
107-
}
108-
finally {
109-
_gate.ExitReadLock();
82+
// Snapshot under the same lock that protects writes to maintain no-post-stop recording guarantee.
83+
List<QueryEvent> snapshot;
84+
lock (_sync) {
85+
snapshot = new List<QueryEvent>(_events);
11086
}
87+
88+
return QueryWatchReport.CreateSnapshot(snapshot, Options, StartedAt, StoppedAt ?? now);
11189
}
11290

113-
/// <summary>
114-
/// Disposes session resources and marks it as stopped.
115-
/// </summary>
91+
/// <summary>Disposes session resources and marks it as stopped.</summary>
11692
public void Dispose() {
11793
// Mark stopped and set StoppedAt once if not set.
11894
if (Interlocked.Exchange(ref _stopped, 1) == 0) {
11995
StoppedAt = DateTimeOffset.UtcNow;
12096
}
121-
12297
_disposed = true;
123-
_gate.Dispose();
12498
}
12599
}
126100
}

0 commit comments

Comments
 (0)