Skip to content

Reduce allocations in NetGetData hot paths#271

Open
Xekep wants to merge 1 commit intoPryaxis:general-develfrom
TerraZ-Team:pr/net-hotpath-optimizations
Open

Reduce allocations in NetGetData hot paths#271
Xekep wants to merge 1 commit intoPryaxis:general-develfrom
TerraZ-Team:pr/net-hotpath-optimizations

Conversation

@Xekep
Copy link

@Xekep Xekep commented Mar 8, 2026

This PR reduces allocations in a few common NetGetData paths without changing the public API.

Changes:

  • replace Enum.IsDefined in NetHooks.OnReceiveData with a cached packet lookup
  • skip GetDataEventArgs allocation when there are no NetGetData handlers
  • cache the net text module id
  • avoid extra array/string work when handling ClientUUID
  • use a bounded packet span check before reading packet payloads

Why:
NetGetData is on a very hot path. The current code does extra allocations and repeated enum checks even when the packet is simple or when no plugin handles NetGetData.

Measurements:

  • ClientUUID: 0.331M ops/s -> 1.805M ops/s (5.45x faster)
  • ClientUUID allocations: 35.137 GB -> 1.319 GB over 3,000,000 iterations
  • packet with no NetGetData handlers: 23.248M ops/s -> 77.314M ops/s (3.33x faster)
  • no-handler packet allocations: 228.882 MB -> ~0 MB over 5,000,000 iterations

Validation:

  • dotnet build TSAPI.sln -c Release
  • dotnet test TSAPI.sln -c Release --no-build --filter FullyQualifiedName!~Benchmarks

@hakusaro hakusaro requested a review from sgkoishi March 9, 2026 07:24
Copy link
Member

@sgkoishi sgkoishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use real profiling data rather than crafted benchmarks that don't reflect the actual use case. Most if not all of these PRs are examples of premature optimization.

return;
}
if (!Enum.IsDefined(typeof(PacketTypes), (int)e.PacketId))
if ((uint)e.PacketId >= knownPacketIds.Length || !knownPacketIds.Get(e.PacketId))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enum.IsDefined itself takes <2ns and replacing it with BitArray won't make a difference (especially when this snippet is executed <100 times per second)

SHA512 shaM = new SHA512Managed();
var result = shaM.ComputeHash(uuid);
Netplay.Clients[buffer.whoAmI].ClientUUID = result.Aggregate("", (s, b) => s + b.ToString("X2"));
Netplay.Clients[buffer.whoAmI].ClientUUID = Convert.ToHexString(SHA512.HashData(uuidBytes));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only line worth merging that actually improves (in terms of readability). That said, although the current version is an anti-pattern, the performance 'improvement' 0.331M ops/s -> 1.805M ops/s is still irrelevant: we'll never even get close to 0.00001M ops/s.

@bartico6
Copy link
Contributor

@sgkoishi I'm thinking there's a non-zero chance he's optimising from real-world attacks on his server he witnessed - with running a large server of his own and all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants