From 331492022a3fbc9bf27a302b3b8ccabbebfe9928 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 7 Aug 2026 21:57:10 +0100 Subject: [PATCH] The classic leg used ONE connection whatever -c said; give each client its own readme.md documents `+m`/`-m` as "when enabled clients share a connection, otherwise each client has a separate connection". The classic (old-core) leg did not implement the second half: OldCoreBenchmarkBase built a single ConnectionMultiplexer in its constructor and `GetClient(int index)` ignored the index entirely, returning that one IDatabase to every worker. `Multiplexed` was never read on this leg at all -- `+m` parsed and then did nothing. So `-c` was CONCURRENCY (in-flight requests pipelined onto one connection), not connection count, on the leg that ships. The fan-out above it was always correct: BenchmarkBase allocates ClientCount tasks and calls GetClient(i) per worker; only the worker-to-connection mapping collapsed. A multiplexer is one connection per endpoint by design, so "a connection per client" means a multiplexer per client. Now builds `Multiplexed ? 1 : ClientCount` of them and maps worker i to `i % connectionCount`, so `+m` reproduces the previous behaviour exactly and is the only way to get it. NewCoreBenchmark already did this (`new(count: Multiplexed ? 1 : ClientCount)`); BridgeBenchmark shares this base and inherits the fix. VERIFIED against a live server, sampling DURING the run from two independent vantage points -- the OS TCP table by owning pid, and the server's own CLIENT LIST: -c 1 OS 1,1,1,1 CLIENT LIST 1 (+ the cli itself) -c 8 OS 8,8,8,8 CLIENT LIST 8 -c 50 OS 50,50,50,50 CLIENT LIST 50 -c 50 +m OS 1,1,1,1 CLIENT LIST 1 The `+m` cell is the discriminating one: without it, a fix that simply hard-wired N connections would pass just as happily. `--basic` also exercised (exit 0). AND A BANNER THAT CANNOT AGREE WITH A FLAG THAT DID NOTHING. The per-test banner printed ", mux" whenever `+m` was passed -- including on the leg where it had no effect -- so it reported the FLAG, not the behaviour, and could not have caught this. `ConnectionCount` is virtual, defaulting to the intended count, and OldCoreBenchmarkBase overrides it with `_connectionMultiplexers.Length`: the number actually created. It is printed on the `### ... ###` announce line, which survives `-q` so a harness can gate on it, and in the per-test banner. --- src/RESPite.Benchmark/BenchmarkBase.cs | 8 ++++- src/RESPite.Benchmark/OldCoreBenchmarkBase.cs | 32 +++++++++++++++---- src/RESPite.Benchmark/Program.cs | 3 +- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/RESPite.Benchmark/BenchmarkBase.cs b/src/RESPite.Benchmark/BenchmarkBase.cs index 6e08b0515..92c399232 100644 --- a/src/RESPite.Benchmark/BenchmarkBase.cs +++ b/src/RESPite.Benchmark/BenchmarkBase.cs @@ -50,6 +50,12 @@ public enum PipelineStrategy public bool Loop { get; } public bool Quiet { get; } public int ClientCount { get; } = 50; + + // the number of connections in use; implementations that create their connections eagerly override + // this with the number they ACTUALLY created, rather than the number the flags asked for -- so that + // a banner cannot report a connection topology that the implementation did not actually deliver + public virtual int ConnectionCount => Multiplexed ? 1 : ClientCount; + private int _operationsPerClient; public int OperationsPerClient(int divisor = 1) => _operationsPerClient / divisor; @@ -486,7 +492,7 @@ private async Task RunAsyncCore( else { Console.Write( - $"====== {name}{description}{auxReason} ====== (clients: {ClientCount:#,##0}, ops: {TotalOperations(divisor):#,##0}"); + $"====== {name}{description}{auxReason} ====== (clients: {ClientCount:#,##0}, conns: {ConnectionCount:#,##0}, ops: {TotalOperations(divisor):#,##0}"); if (Multiplexed) { Console.Write(", mux"); diff --git a/src/RESPite.Benchmark/OldCoreBenchmarkBase.cs b/src/RESPite.Benchmark/OldCoreBenchmarkBase.cs index a124d9e4b..958ee65d3 100644 --- a/src/RESPite.Benchmark/OldCoreBenchmarkBase.cs +++ b/src/RESPite.Benchmark/OldCoreBenchmarkBase.cs @@ -9,15 +9,28 @@ namespace RESPite.Benchmark; public abstract class OldCoreBenchmarkBase : BenchmarkBase { - private readonly IConnectionMultiplexer _connectionMultiplexer; - private readonly IDatabase _client; + private readonly IConnectionMultiplexer[] _connectionMultiplexers; + private readonly IDatabase[] _clients; private readonly KeyValuePair[] _pairs; public OldCoreBenchmarkBase(string[] args) : base(args) { - // ReSharper disable once VirtualMemberCallInConstructor - _connectionMultiplexer = Create(Port); - _client = _connectionMultiplexer.GetDatabase(); + // a multiplexer is one connection per endpoint by design, so "each client has a separate + // connection" means a multiplexer each; +m is the documented opt-out, sharing a single one + var connectionCount = Multiplexed ? 1 : ClientCount; + _connectionMultiplexers = new IConnectionMultiplexer[connectionCount]; + for (var i = 0; i < connectionCount; i++) + { + // ReSharper disable once VirtualMemberCallInConstructor + _connectionMultiplexers[i] = Create(Port); + } + + _clients = new IDatabase[ClientCount]; + for (var i = 0; i < ClientCount; i++) + { + _clients[i] = _connectionMultiplexers[i % connectionCount].GetDatabase(); + } + _pairs = new KeyValuePair[10]; for (var i = 0; i < 10; i++) @@ -26,6 +39,8 @@ public OldCoreBenchmarkBase(string[] args) : base(args) } } + public override int ConnectionCount => _connectionMultiplexers.Length; + protected abstract IConnectionMultiplexer Create(int port); protected override async Task OnCleanupAsync(IDatabaseAsync client) @@ -40,10 +55,13 @@ protected override async Task OnCleanupAsync(IDatabaseAsync client) public override void Dispose() { - _connectionMultiplexer.Dispose(); + foreach (var connectionMultiplexer in _connectionMultiplexers) + { + connectionMultiplexer.Dispose(); + } } - protected override IDatabaseAsync GetClient(int index) => _client; + protected override IDatabaseAsync GetClient(int index) => _clients[index]; protected override Task DeleteAsync(IDatabaseAsync client, string key) => client.KeyDeleteAsync(key); public override async Task RunAll() diff --git a/src/RESPite.Benchmark/Program.cs b/src/RESPite.Benchmark/Program.cs index a0797b787..287f6ba52 100644 --- a/src/RESPite.Benchmark/Program.cs +++ b/src/RESPite.Benchmark/Program.cs @@ -53,7 +53,8 @@ private static async Task Main(string[] args) { if (benchmarks.Count > 1 || isFirst) { - Console.WriteLine($"### {bench} ###"); + Console.WriteLine( + $"### {bench} (clients: {bench.ClientCount:#,##0}, conns: {bench.ConnectionCount:#,##0}) ###"); isFirst = false; }