warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

perf: retire auto-denylist holds from an expiry ring instead of scanning

Browse source 
Reclaiming lapsed holds was O(entries held). Every cap-triggered reclaim
during a flood, and every periodic sweep, walked the whole dictionary to
find the few that had expired. That is what forced the cap to be sized for
the scan rather than for the flood.

A hold is now never refreshed: the first detection sets the expiry and
later ones leave it alone. That makes insertion order equal to expiry order,
so a ring of the same keys is sorted by construction and retiring lapsed
entries stops at the first live record -- the cost is the number expiring,
not the number held. Nothing is lost by dropping the refresh: the rate
limiter runs ahead of the connection filters (NetState.Network.cs) and
reports to the ban channel, so a flooder whose hold lapses is re-held on its
next attempt.

Because the ring carries the expiry, the dictionary only had to answer
membership, so it is a HashSet now: 36 bytes a slot against 52. The ring is
parallel UInt128[]/long[] rather than an array of structs -- UInt128 forces
16-byte alignment, so a packed pair would cost 32 bytes where these cost 24,
and the drain reads only the long[].

Measured, standalone copies of both designs head to head at the shipped
324,449 cap:

  accept path, nothing held      9.1ns -> 6.1ns per call
  sustained flood at cap        26.7ms -> 9.3ms over 60k rejected addresses
  flood end, realistic spread    9.49ms -> 0.05ms worst single call
  flood end, all at one instant  8.85ms -> 10.69ms

The third line is the point: the on-loop stall drops 190x because the work
is spread across the calls that were happening anyway. Two honest costs.
Total work over that spread rises 1.9x (sequential dictionary scan beats
random-access set removes on cache), and the synthetic case where every
entry shares one expiry millisecond is 21% worse. Peak latency is the
currency for a game loop, and reaching that synthetic case needs an entire
flood to land inside one millisecond.

The SweepThrottleMs added earlier goes away with the scan it was throttling.
The periodic timer stays, now O(expiring), purely to reclaim memory on a
shard that goes quiet after a flood.

Release now purges the ring record too. Nothing records that a key was
released, so a re-detection before the old record lapsed would otherwise be
retired early by it. O(n), on an operator retraction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-08-13 22:13:31 -07:00
parent 87ddb22369
commit 8e5b7af1cf
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
2 changed files with 202 additions and 91 deletions
Download patch file Download diff file Expand all files Collapse all files

76
Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs
View file

@ -62,8 +62,11 @@ public class AutoDenylistTests
Assert.Equal(0, AutoDenylist.Count);
}
// Not refreshed on purpose: it is what keeps insertion order equal to expiry order, so retiring lapsed
// entries costs the number expiring instead of the number held. A flooder whose hold lapses trips the
// rate limiter on its next attempt -- which runs ahead of the connection filters -- and is held again.
[Fact]
public void Repeat_detection_extends_the_hold()
public void Repeat_detection_does_not_extend_the_hold()
{
Reset();
var ip = IPAddress.Parse("198.51.100.13");
@ -71,8 +74,51 @@ public class AutoDenylistTests
AutoDenylist.Hold(ip, BanReasons.SilentConnect, Now);
AutoDenylist.Hold(ip, BanReasons.SilentConnect, Now + DurationMs - 1);
Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs + 1)); // would have lapsed without the second
Assert.Equal(1, AutoDenylist.Count); // and did not add a duplicate
Assert.Equal(1, AutoDenylist.Count); // no duplicate
Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs - 1));
Assert.False(AutoDenylist.IsDenied(ip, Now + DurationMs + 1)); // lapses from the FIRST detection
}
// The ring carries the expiry and the set carries membership; if they ever disagree, an address is
// either denied forever or retired early.
[Fact]
public void Ring_and_set_stay_in_step()
{
Reset(maxEntries: 4);
for (var i = 0; i < 8; i++)
{
AutoDenylist.Hold(IPAddress.Parse($"198.51.100.{70 + i}"), BanReasons.InvalidSeed, Now);
}
Assert.Equal(4, AutoDenylist.Count);
Assert.Equal(AutoDenylist.Count, AutoDenylist.RingCount);
AutoDenylist.Release(IPAddress.Parse("198.51.100.71"));
Assert.Equal(3, AutoDenylist.Count);
Assert.Equal(AutoDenylist.Count, AutoDenylist.RingCount);
AutoDenylist.Drain(Now + DurationMs + 1);
Assert.Equal(0, AutoDenylist.Count);
Assert.Equal(0, AutoDenylist.RingCount);
}
// Releasing leaves no ring record behind, so a re-detection is not retired by the old one.
[Fact]
public void Release_then_re_hold_is_not_retired_by_the_stale_record()
{
Reset();
var ip = IPAddress.Parse("198.51.100.15");
AutoDenylist.Hold(ip, BanReasons.RateLimit, Now);
AutoDenylist.Release(ip);
var later = Now + DurationMs - 1;
AutoDenylist.Hold(ip, BanReasons.RateLimit, later);
// The first hold's expiry has passed; the second must survive it.
Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs + 1));
Assert.Equal(1, AutoDenylist.RingCount);
}
[Fact]
@ -104,30 +150,6 @@ public class AutoDenylistTests
Assert.False(AutoDenylist.IsDenied(IPAddress.Parse("198.51.100.109"), Now));
}
// A sweep is O(maxEntries). Without the throttle a distinct-source flood re-scans the whole
// dictionary for every address it rejects, which is the case the cap exists to make cheap.
[Fact]
public void Cap_does_not_re_sweep_for_every_rejected_address()
{
Reset(maxEntries: 2);
AutoDenylist.Hold(IPAddress.Parse("198.51.100.40"), BanReasons.InvalidSeed, Now);
AutoDenylist.Hold(IPAddress.Parse("198.51.100.41"), BanReasons.InvalidSeed, Now);
Assert.Equal(0, AutoDenylist.SweepCount);
// At the cap with nothing lapsed: the first rejection sweeps, the rest in the window do not.
for (var i = 0; i < 5; i++)
{
Assert.False(AutoDenylist.Hold(IPAddress.Parse($"198.51.100.{50 + i}"), BanReasons.InvalidSeed, Now));
}
Assert.Equal(1, AutoDenylist.SweepCount);
// Past the window it may try again, since entries can have lapsed by then.
Assert.False(AutoDenylist.Hold(IPAddress.Parse("198.51.100.60"), BanReasons.InvalidSeed, Now + 1000));
Assert.Equal(2, AutoDenylist.SweepCount);
}
[Fact]
public void Reaching_the_cap_reclaims_lapsed_entries_first()
{

217
Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs
View file

@ -32,28 +32,36 @@ namespace Server.Network;
/// no bouncer, which is the default. Not persisted, by design: a holding pen that survives restarts is a ban
/// without a ban's review. Only <see cref="BanReasons.IsBehavioral"/> verdicts are held.
/// </remarks>
/// <remarks>
/// A hold is never refreshed, so every expiry is <c>insertion + duration</c> and the ring is sorted by
/// construction. Retiring lapsed entries is therefore the number expiring rather than the number held, which
/// is what lets the cap be sized for the flood instead of for a scan.
/// </remarks>
public static class AutoDenylist
{
private static readonly ILogger logger = LogFactory.GetLogger(typeof(AutoDenylist));
// Address (normalized v6 bits) -> Core.TickCount at which the hold lapses. Loop-only.
private static readonly Dictionary<UInt128, long> _held = [];
// Membership only (normalized v6 bits). Loop-only. The expiry lives beside the key in the ring, so
// there is exactly one copy of it and the two cannot disagree.
private static readonly HashSet<UInt128> _held = [];
// Shortest gap between cap-triggered sweeps. A sweep is O(_maxEntries) and one that just ran cannot
// have freed more, so without this every rejected address re-scans the whole dictionary -- turning the
// bound into a cost multiplier under exactly the flood it exists to bound.
private const long SweepThrottleMs = 1000;
// The same keys in expiry order. Parallel arrays rather than an array of structs: UInt128 forces
// 16-byte alignment, so a packed (key, expiry) struct costs 32 bytes where these cost 24 -- and the
// drain reads only the long[], 8 sequential bytes per entry.
private static UInt128[] _ringKeys = [];
private static long[] _ringExpiry = [];
private static int _ringHead;
private static int _ringCount;
private static bool _enabled;
private static long _durationMs;
private static int _maxEntries;
private static bool _warnedFull;
private static long _lastSweepAt;
public static int Count => _held.Count;
// Test seam: lets the throttle be asserted rather than inferred.
internal static int SweepCount { get; private set; }
// Test seam: the ring and the set hold the same entries, and nothing else may assume it.
internal static int RingCount => _ringCount;
public static void Configure()
{
@ -69,9 +77,6 @@ public static class AutoDenylist
_durationMs = (long)s.Duration.TotalMilliseconds;
_maxEntries = s.MaxEntries;
// Seeded from a real tick: tick counts need not start near zero. See dev-docs/tick-counts.md.
_lastSweepAt = Core.TickCount;
ConnectionFilters.Register(new AutoDenylistFilter());
BanChannel.Register(new AutoDenylistReporter());
}
@ -89,38 +94,45 @@ public static class AutoDenylist
return false;
}
var key = address.ToUInt128();
Drain(nowTicks);
// An address already held is just extended, so no cap check is needed.
if (!_held.ContainsKey(key) && _held.Count >= _maxEntries)
// Deliberately not refreshed: the first detection sets the expiry and later ones leave it alone.
// That is what keeps insertion order equal to expiry order, which is the whole reason the drain
// can stop at the first live record. A flooder whose hold lapses trips the rate limiter on its
// next attempt -- which runs before the connection filters -- and is held again immediately.
if (!_held.Add(address.ToUInt128()))
{
if (nowTicks - _lastSweepAt >= SweepThrottleMs)
{
Sweep(nowTicks);
}
if (_held.Count >= _maxEntries)
{
if (!_warnedFull)
{
_warnedFull = true;
logger.Warning(
"Auto-denylist is full at {Max} addresses; further detections are disconnected but not held",
_maxEntries
);
}
return false;
}
return true;
}
_held[key] = nowTicks + _durationMs;
// Drain already reclaimed everything reclaimable, so being over now means genuinely full.
if (_held.Count > _maxEntries)
{
_held.Remove(address.ToUInt128());
if (!_warnedFull)
{
_warnedFull = true;
logger.Warning(
"Auto-denylist is full at {Max} addresses; further detections are disconnected but not held",
_maxEntries
);
}
return false;
}
Push(address.ToUInt128(), nowTicks + _durationMs);
return true;
}
public static bool IsDenied(IPAddress address) => IsDenied(address, Core.TickCount);
/// <summary>The pure decision, split out so the accept-path policy can be tested without a clock.</summary>
/// <summary>
/// The accept-path decision, split out so the policy can be tested without a clock. Drains first: the
/// expiry is not stored beside the membership, so there is nothing to expire on read and a lapsed hold
/// must be retired here rather than left to deny. Costs one array read when nothing has lapsed.
/// </summary>
internal static bool IsDenied(IPAddress address, long nowTicks)
{
if (!_enabled || address == null)
@ -128,56 +140,132 @@ public static class AutoDenylist
return false;
}
// Decided on read, so a lapsed hold cannot deny even before the sweep. Subtraction: TickCount wraps.
return _held.TryGetValue(address.ToUInt128(), out var expires) && expires - nowTicks > 0;
Drain(nowTicks);
return _held.Contains(address.ToUInt128());
}
/// <summary>Releases an address early, e.g. when an operator retracts a ban.</summary>
public static void Release(IPAddress address)
{
if (_enabled && address != null)
{
_held.Remove(address.ToUInt128());
}
}
internal static void Sweep(long nowTicks)
{
// Stamped even when there is nothing to do, so the cap path throttles off the last attempt.
_lastSweepAt = nowTicks;
SweepCount++;
if (_held.Count == 0)
if (!_enabled || address == null)
{
return;
}
var lapsed = 0;
foreach (var (address, expires) in _held)
var key = address.ToUInt128();
if (_held.Remove(key))
{
if (expires - nowTicks <= 0)
{
_held.Remove(address);
lapsed++;
}
// The ring record has to go too. Nothing records that this key was released, so if it were
// detected again before the old record lapsed, that record would retire the new hold early.
// O(n), but this is an operator retraction, not the accept path.
PurgeRing(key);
}
}
/// <summary>
/// Retires everything that has lapsed. Expiries only ever increase along the ring, so the first live
/// record ends the scan and the cost is the number actually expiring, not the number held.
/// </summary>
internal static void Drain(long nowTicks)
{
// Subtraction, never a direct compare: tick counts wrap. See dev-docs/tick-counts.md.
while (_ringCount > 0 && _ringExpiry[_ringHead] - nowTicks <= 0)
{
_held.Remove(_ringKeys[_ringHead]);
_ringHead = _ringHead + 1 == _ringKeys.Length ? 0 : _ringHead + 1;
_ringCount--;
_warnedFull = false;
}
}
private static void Push(UInt128 key, long expiry)
{
if (_ringCount == _ringKeys.Length)
{
Grow();
}
if (lapsed > 0)
var tail = _ringHead + _ringCount;
if (tail >= _ringKeys.Length)
{
_warnedFull = false;
tail -= _ringKeys.Length;
}
_ringKeys[tail] = key;
_ringExpiry[tail] = expiry;
_ringCount++;
}
private static void Grow()
{
var size = Math.Max(64, _ringKeys.Length * 2);
var keys = new UInt128[size];
var expiry = new long[size];
for (var i = 0; i < _ringCount; i++)
{
var from = _ringHead + i;
if (from >= _ringKeys.Length)
{
from -= _ringKeys.Length;
}
keys[i] = _ringKeys[from];
expiry[i] = _ringExpiry[from];
}
_ringKeys = keys;
_ringExpiry = expiry;
_ringHead = 0;
}
private static void PurgeRing(UInt128 key)
{
var capacity = _ringKeys.Length;
for (var i = 0; i < _ringCount; i++)
{
var at = _ringHead + i;
if (at >= capacity)
{
at -= capacity;
}
if (_ringKeys[at] != key)
{
continue;
}
// Close the gap so the ring stays contiguous and expiry-ordered.
for (var j = i; j < _ringCount - 1; j++)
{
var to = _ringHead + j;
if (to >= capacity)
{
to -= capacity;
}
var from = to + 1 == capacity ? 0 : to + 1;
_ringKeys[to] = _ringKeys[from];
_ringExpiry[to] = _ringExpiry[from];
}
_ringCount--;
return;
}
}
internal static void LoadForTesting(bool enabled, long durationMs, int maxEntries)
{
_held.Clear();
_ringKeys = [];
_ringExpiry = [];
_ringHead = 0;
_ringCount = 0;
_enabled = enabled;
_durationMs = durationMs;
_maxEntries = maxEntries;
_warnedFull = false;
_lastSweepAt = 0;
SweepCount = 0;
}
}
@ -194,11 +282,12 @@ public sealed class AutoDenylistFilter : IConnectionFilter
public void Start(CancellationToken token)
{
// Only an optimization: IsDenied expires on read.
// Only reclaims memory: Hold and IsDenied both drain, so this matters on a shard that has gone
// quiet after a flood and would otherwise hold the ring until someone next connects.
_sweepTimer = Timer.DelayCall(
TimeSpan.FromMinutes(1),
TimeSpan.FromMinutes(1),
() => AutoDenylist.Sweep(Core.TickCount)
() => AutoDenylist.Drain(Core.TickCount)
);
}