Skip to content

deleted #2809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Arufonsu wants to merge 1 commit into from
Closed

deleted #2809

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions Intersect.Server.Core/Core/LogicService.LogicThread.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Intersect.Server.Metrics;
using Intersect.Server.Networking;
using Intersect.Server.Database.PlayerData.Players;
using Intersect.Utilities;
using Intersect.Server.Database.PlayerData.Api;
using Intersect.Server.Core.MapInstancing;

Expand Down Expand Up @@ -68,6 +67,12 @@ public LogicThread(LogicService logicService) : base("ServerLogic")
/// </summary>
public readonly Dictionary<Guid, MapInstance> ActiveMapInstances = new Dictionary<Guid, MapInstance>();

// Reusable buffer for UpdateInstanceControllers — avoids ToArray() every 250ms.
private MapInstance[] _activeMapInstancesBuffer = Array.Empty<MapInstance>();

// Track last console title to avoid redundant syscalls every second.
private string _lastConsoleTitle = string.Empty;

protected override void ThreadStart(ServerContext serverContext)
{
if (serverContext == null)
Expand All @@ -87,6 +92,7 @@ protected override void ThreadStart(ServerContext serverContext)

var processedMapInstances = new HashSet<Guid>();
var sourceMapInstances = new HashSet<Guid>();
var toRemove = new List<Guid>();
var players = 0;

while (ServerContext.Instance.IsRunning)
Expand Down Expand Up @@ -128,7 +134,12 @@ protected override void ThreadStart(ServerContext serverContext)
if (Options.Instance.Metrics.Enable)
{
events += player.EventLookup.Count;
eventsProcessing += player.EventLookup.Values.Where(e => e.CallStack?.Count > 0).Count();

foreach (var evt in player.EventLookup.Values)
{
if (evt.CallStack?.Count > 0) eventsProcessing++;
}

autorunEvents += player.CommonAutorunEvents + player.MapAutorunEvents;
}
}
Expand All @@ -143,7 +154,7 @@ protected override void ThreadStart(ServerContext serverContext)
{
if (!processedMapInstances.Contains(instance.Id))
{
if (!ActiveMapInstances.Keys.Contains(instance.Id))
if (!ActiveMapInstances.ContainsKey(instance.Id)) // ContainsKey() directly instead of .Keys.Contains().
{
AddToQueue(instance);
}
Expand All @@ -157,8 +168,9 @@ protected override void ThreadStart(ServerContext serverContext)
}
}

//Refresh list of active maps & their instances
Copy link
Member

@pandinocoder pandinocoder Feb 22, 2026

Choose a reason for hiding this comment

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

The comment should be kept (the new comment is also fine, you should have both)

foreach (var (instanceId, mapInstance) in ActiveMapInstances.ToArray())
Copy link
Member

@pandinocoder pandinocoder Feb 22, 2026

Choose a reason for hiding this comment

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

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?

// Iterate dictionary directly; collect removals in reusable list.
toRemove.Clear();
foreach (var (instanceId, mapInstance) in ActiveMapInstances)
{
if (processedMapInstances.Contains(instanceId) || mapInstance.ShouldBeActive())
{
Expand All @@ -176,12 +188,23 @@ protected override void ThreadStart(ServerContext serverContext)
mapInstance.ResetNpcSpawns();
}
}
toRemove.Add(instanceId);
}

ActiveMapInstances.Remove(instanceId);
foreach (var id in toRemove)
{
ActiveMapInstances.Remove(id);
}

// Allow our instance controllers to keep their instances up to date
Copy link
Member

@pandinocoder pandinocoder Feb 22, 2026

Choose a reason for hiding this comment

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

Should have both comments

InstanceProcessor.UpdateInstanceControllers(ActiveMapInstances.Values.ToArray());
// Reuse buffer; only reallocate when count changes.
var activeCount = ActiveMapInstances.Count;
if (_activeMapInstancesBuffer.Length != activeCount)
{
_activeMapInstancesBuffer = new MapInstance[activeCount];
}

ActiveMapInstances.Values.CopyTo(_activeMapInstancesBuffer, 0);
InstanceProcessor.UpdateInstanceControllers(_activeMapInstancesBuffer);
Comment on lines +200 to +207
Copy link
Member

@pandinocoder pandinocoder Feb 22, 2026

Choose a reason for hiding this comment

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

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.


if (Options.Instance.Metrics.Enable)
{
Expand Down Expand Up @@ -237,7 +260,14 @@ protected override void ThreadStart(ServerContext serverContext)
swCps = 0;

var cyclesPerSecond = ApplicationContext.GetCurrentContext<IServerContext>().LogicService.CyclesPerSecond;
Console.Title = $"Intersect Server - CPS: {cyclesPerSecond}, Players: {players}, Active Maps: {ActiveMapInstances.Count}, Logic Threads: {LogicPool.ActiveThreads} ({LogicPool.InUseThreads} In Use), Pool Queue: {LogicPool.CurrentWorkItemsCount}, Idle: {LogicPool.IsIdle}";

// Only call Console.Title when the value actually changed.
var newTitle = $"Intersect Server - CPS: {cyclesPerSecond}, Players: {players}, Active Maps: {ActiveMapInstances.Count}, Logic Threads: {LogicPool.ActiveThreads} ({LogicPool.InUseThreads} In Use), Pool Queue: {LogicPool.CurrentWorkItemsCount}, Idle: {LogicPool.IsIdle}";
if (newTitle != _lastConsoleTitle)
{
Console.Title = newTitle;
_lastConsoleTitle = newTitle;
}

if (Options.Instance.Metrics.Enable)
{
Expand Down Expand Up @@ -368,7 +398,7 @@ private void UpdateMap(MapInstance mapInstance, bool onlyProjectiles)
if (onlyProjectiles)
{
mapInstance.UpdateProjectiles(Timing.Global.Milliseconds);
if (ActiveMapInstances.Keys.Contains(mapInstance.Id))
if (ActiveMapInstances.ContainsKey(mapInstance.Id))
{
MapInstanceProjectileQueue.Enqueue(mapInstance);
}
Expand All @@ -392,7 +422,7 @@ private void UpdateMap(MapInstance mapInstance, bool onlyProjectiles)
mapInstance.Update(Timing.Global.Milliseconds);
}

if (ActiveMapInstances.Keys.Contains(mapInstance.Id))
if (ActiveMapInstances.ContainsKey(mapInstance.Id))
{
MapInstanceUpdateQueue.Enqueue(mapInstance);
}
Expand Down
Loading