Skip to content

Improve Multi-threaded landblock group ticking (non-physics) - #2383

Merged
Mag-nus merged 11 commits into
ACEmulator:masterfrom
Mag-nus:safer-multi-threading
Nov 17, 2019
Merged

Mag-nus merged 11 commits into
ACEmulator:masterfrom
Mag-nus:safer-multi-threading

Conversation

@Mag-nus

@Mag-nus Mag-nus commented Nov 4, 2019

Copy link
Copy Markdown
Member

This splits up the previous Landblock Tick() work into two functions:
TickMultiThreadedWork()
TickSingleThreadedWork()

Mainly, this allows multi-threading of the following work:
Landblock.actionQueue.RunActions();
Landblock creatures Monster_Tick()
Landblock Heartbeat
Landblock Database Save

The first two of which are the biggest CPU consumers from the previous Tick(), and, fortunately enough are pretty safe to multi-thread.

This splits up the previous Landblock Tick() work into two functions:
TickMultiThreadedWork()
TickSingleThreadedWork()

Mainly, this allows multi-threading of the following work:
Landblock.actionQueue.RunActions();
Landblock creatures Monster_Tick()
Landblock Heartbeat
Landblock Database Save

The first two of which are the biggest CPU consumers from the previous Tick(), and, fortunately enough are pretty safe to multi-thread.

@gmriggs gmriggs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ce has been running this pr, no issues reported

@gmriggs

gmriggs commented Nov 14, 2019

Copy link
Copy Markdown
Collaborator

ce has been running this PR without any issues for the past few days, however when we tried to add in #2415 today, the server crashed almost immediately on startup with this error:

Unhandled exception. System.AggregateException: One or more errors occurred. (An item with the same key has already been added. Key: 150995130)
 ---> System.ArgumentException: An item with the same key has already been added. Key: 150995130
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at ACE.Server.Physics.Animation.MotionTable.GetRunSpeed(UInt32 motionTableID) in /home/gmriggs/ACE/ACE/Source/ACE.Server/Physics/Animation/MotionTable.cs:line 503
   at ACE.Server.WorldObjects.Creature.GetMovementSpeed() in /home/gmriggs/ACE/ACE/Source/ACE.Server/WorldObjects/Monster_Navigation.cs:line 336
   at ACE.Server.WorldObjects.Creature.StartTurn() in /home/gmriggs/ACE/ACE/Source/ACE.Server/WorldObjects/Monster_Navigation.cs:line 87
   at ACE.Server.WorldObjects.Creature.Monster_Tick(Double currentUnixTime) in /home/gmriggs/ACE/ACE/Source/ACE.Server/WorldObjects/Monster_Tick.cs:line 157
   at ACE.Server.Entity.Landblock.TickMultiThreadedWork(Double currentUnixTime) in /home/gmriggs/ACE/ACE/Source/ACE.Server/Entity/Landblock.cs:line 452
   at ACE.Server.Managers.LandblockManager.<>c.<TickMultiThreadedWork>b__15_0(LandblockGroup landblockGroup) in /home/gmriggs/ACE/ACE/Source/ACE.Server/Managers/LandblockManager.cs:line 306
   at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`1.<ForWorker>b__1(RangeWorker& currentWorker, Int32 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)

2415 adds some new logic for players, but the calling profile of these functions should not be affected. this error was also purely for the monster movement paths, which 2415 does not change

i'm wondering if it could have just been an odd coincidence that this happened when 2415 was added in... the way i see this error happening is right when the server is first starting up, if 2 monsters of the same type in separate landblocks both start running around the exact same time, this error could maybe occur

@Mag-nus
Mag-nus merged commit df0b662 into ACEmulator:master Nov 17, 2019
@Mag-nus
Mag-nus deleted the safer-multi-threading branch November 17, 2019 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants