performance: reduce server allocations in LogicThread hot path#2809
performance: reduce server allocations in LogicThread hot path#2809
Conversation
Reduces several per-tick and per-second heap allocations in LogicService.LogicThread to reduce GC pressure at runtime. - Replace ActiveMapInstances.ToArray() with a reusable MapInstance[] buffer (_activeMapInstancesBuffer); reallocate only when Count changes - Collect map instance removals in a reusable List<Guid> (toRemove) declared outside the while-loop instead of removing during enumeration - Replace .Where(...).Count() LINQ chain with a plain foreach loop when counting active event call stacks in the metrics block - Replace ActiveMapInstances.Keys.Contains() with ContainsKey() in all three call sites (direct dictionary lookup, O(1) vs O(n)) - Guard Console.Title assignment behind a string equality check using _lastConsoleTitle; avoids redundant Win32/syscall every second when server state is unchanged - Remove unused `using Intersect.Utilities` import Signed-off-by: Arufonsu <17498701+Arufonsu@users.noreply.github.com>
Arufonsu
added
chore
| } | ||
| } | ||
|
|
||
| //Refresh list of active maps & their instances |
There was a problem hiding this comment.
The comment should be kept (the new comment is also fine, you should have both)
| } | ||
|
|
||
| //Refresh list of active maps & their instances | ||
| foreach (var (instanceId, mapInstance) in ActiveMapInstances.ToArray()) |
There was a problem hiding this comment.
The only thing is that the new condition isn't safe against race conditions, even if this thread isn't changing the collection, another thread can. Is the collection only editable by this thread?
| ActiveMapInstances.Remove(id); | ||
| } | ||
|
|
||
| // Allow our instance controllers to keep their instances up to date |
| var activeCount = ActiveMapInstances.Count; | ||
| if (_activeMapInstancesBuffer.Length != activeCount) | ||
| { | ||
| _activeMapInstancesBuffer = new MapInstance[activeCount]; | ||
| } | ||
|
|
||
| ActiveMapInstances.Values.CopyTo(_activeMapInstancesBuffer, 0); | ||
| InstanceProcessor.UpdateInstanceControllers(_activeMapInstancesBuffer); |
There was a problem hiding this comment.
Better way of doing this is to change UpdateInstanceControllers() to take in a second default paramter int count = -1, and if count is -1 then change it to the length of the passed in array.
If count is 0 or positive, then loop through only those elements.
Why?
So then you only reallocate when the new count is larger (or significantly smaller).
Larger is simple, if it needs even 1 more you resize, but maybe resize with a buffer of 10%.
Significantly smaller can be defined as 45% or less of the current size (you still want a 10% buffer, that would get to around 50% of the current length taking that into account) but it also has to be a difference of at least 128 elements before you will shrink (if you go too low then it will constantly reallocate when you only have a few maps, and the cost of the array isn't high).
The only caveat is that you also need to clear "unused" elements in the array to null so that you don't make some elements live past their lifetime.
2 participants
Reduces several per-tick and per-second heap allocations in LogicService.LogicThread to reduce GC pressure at runtime.
using Intersect.Utilitiesimport