From 45694eea082fd2878e841ae83f49b285aeb00f8c Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 11:37:12 -0400 Subject: [PATCH 1/7] Actors Table fixes and performance improvements --- Dalamud/Game/ClientState/Actors/ActorTable.cs | 97 +++++++++++++------ 1 file changed, 69 insertions(+), 28 deletions(-) diff --git a/Dalamud/Game/ClientState/Actors/ActorTable.cs b/Dalamud/Game/ClientState/Actors/ActorTable.cs index e6e02068d..6369b9c6e 100644 --- a/Dalamud/Game/ClientState/Actors/ActorTable.cs +++ b/Dalamud/Game/ClientState/Actors/ActorTable.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Runtime.InteropServices; using Dalamud.Game.ClientState.Actors.Types; using Dalamud.Game.ClientState.Actors.Types.NonPlayer; @@ -13,10 +14,23 @@ namespace Dalamud.Game.ClientState.Actors { /// /// This collection represents the currently spawned FFXIV actors. /// - public class ActorTable : IReadOnlyCollection, ICollection { + public class ActorTable : IReadOnlyCollection, ICollection, IDisposable { private const int ActorTableLength = 424; + #region Actor Table Cache + private List actorsCache; + private List ActorsCache { + get { + if (actorsCache != null) return actorsCache; + actorsCache = GetActorTable(); + return actorsCache; + } + } + internal void ResetCache() => actorsCache = null; + #endregion + + #region temporary imports for crash workaround [DllImport("kernel32.dll", SetLastError = true)] @@ -32,8 +46,8 @@ namespace Dalamud.Game.ClientState.Actors { private ClientStateAddressResolver Address { get; } private Dalamud dalamud; - private static int sz = Marshal.SizeOf(typeof(Structs.Actor)); - private IntPtr actorMem = Marshal.AllocHGlobal(sz); + private static int actorMemSize = Marshal.SizeOf(typeof(Structs.Actor)); + private IntPtr actorMem { get; set; } = Marshal.AllocHGlobal(actorMemSize); /// /// Set up the actor table collection. @@ -53,47 +67,54 @@ namespace Dalamud.Game.ClientState.Actors { /// at the specified spawn index. [CanBeNull] public Actor this[int index] { - get { - if (index >= Length) - return null; - - var tblIndex = Address.ActorTable + index * 8; - - var offset = Marshal.ReadIntPtr(tblIndex); - - //Log.Debug($"Reading actor {index} at {tblIndex.ToInt64():X} pointing to {offset.ToInt64():X}"); - - if (offset == IntPtr.Zero) - return null; + get => ActorsCache[index]; + } + private Actor ReadActorFromMemory(IntPtr offset) + { + try { // FIXME: hack workaround for trying to access the player on logout, after the main object has been deleted - //var sz = Marshal.SizeOf(typeof(Structs.Actor)); - //var actorMem = Marshal.AllocHGlobal(sz); // we arguably could just reuse this - if (!ReadProcessMemory(Process.GetCurrentProcess().Handle, offset, actorMem, sz, out _)) { + if (!ReadProcessMemory(Process.GetCurrentProcess().Handle, offset, actorMem, actorMemSize, out _)) + { Log.Debug("ActorTable - ReadProcessMemory failed: likely player deletion during logout"); return null; } var actorStruct = Marshal.PtrToStructure(actorMem); - //Marshal.FreeHGlobal(actorMem); - //Log.Debug("ActorTable[{0}]: {1} - {2} - {3}", index, tblIndex.ToString("X"), offset.ToString("X"), - // actorStruct.ObjectKind.ToString()); - switch (actorStruct.ObjectKind) { case ObjectKind.Player: return new PlayerCharacter(offset, actorStruct, this.dalamud); case ObjectKind.BattleNpc: return new BattleNpc(offset, actorStruct, this.dalamud); default: return new Actor(offset, actorStruct, this.dalamud); } } + catch (Exception e) { + Log.Information($"{e}"); + return null; + } + } + + private IntPtr[] GetPointerTable() { + IntPtr[] ret = new IntPtr[ActorTableLength]; + Marshal.Copy(Address.ActorTable, ret, 0, ActorTableLength); + return ret; + } + + private List GetActorTable() { + var actors = new List(); + var ptrTable = GetPointerTable(); + for (int i = 0; i < ActorTableLength; i++) { + if (ptrTable[i] != IntPtr.Zero) { + actors.Add(ReadActorFromMemory(ptrTable[i])); + } else { + actors.Add(null); + } + } + return actors; } public IEnumerator GetEnumerator() { - for (int i=0;i a != null).GetEnumerator(); } IEnumerator IEnumerable.GetEnumerator() { @@ -103,7 +124,7 @@ namespace Dalamud.Game.ClientState.Actors { /// /// The amount of currently spawned actors. /// - public int Length => ActorTableLength; + public int Length => ActorsCache.Count; int IReadOnlyCollection.Count => Length; @@ -119,5 +140,25 @@ namespace Dalamud.Game.ClientState.Actors { index++; } } + + #region IDisposable Pattern + private bool disposed = false; + + private void Dispose(bool disposing) + { + if (disposed) return; + Marshal.FreeHGlobal(actorMem); + disposed = true; + } + + public void Dispose() { + Dispose(true); + GC.SuppressFinalize(this); + } + + ~ActorTable() { + Dispose(false); + } + #endregion } } From 00ee30f0d9dfb771b1f607df063023087df6ea53 Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 11:39:07 -0400 Subject: [PATCH 2/7] Dispose ActorTable on ClientState dispose --- Dalamud/Game/ClientState/ClientState.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Dalamud/Game/ClientState/ClientState.cs b/Dalamud/Game/ClientState/ClientState.cs index b37dc2651..2275345a5 100644 --- a/Dalamud/Game/ClientState/ClientState.cs +++ b/Dalamud/Game/ClientState/ClientState.cs @@ -138,6 +138,7 @@ namespace Dalamud.Game.ClientState public void Dispose() { this.PartyList.Dispose(); this.setupTerritoryTypeHook.Dispose(); + this.Actors.Dispose(); } private void FrameworkOnOnUpdateEvent(Framework framework) { From ebbd79274b8fd139e3cedc0bb76093c2a7261bcf Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 11:44:28 -0400 Subject: [PATCH 3/7] Reset cache before emitting each framework event --- Dalamud/Game/Internal/Framework.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Dalamud/Game/Internal/Framework.cs b/Dalamud/Game/Internal/Framework.cs index e65ead3a0..cdba70a8b 100644 --- a/Dalamud/Game/Internal/Framework.cs +++ b/Dalamud/Game/Internal/Framework.cs @@ -23,6 +23,7 @@ namespace Dalamud.Game.Internal { public event OnUpdateDelegate OnUpdateEvent; private Hook updateHook; + private Dalamud dalamud; /// @@ -49,6 +50,8 @@ namespace Dalamud.Game.Internal { #endregion public Framework(SigScanner scanner, Dalamud dalamud) { + this.dalamud = dalamud; + Address = new FrameworkAddressResolver(); Address.Setup(scanner); @@ -101,6 +104,7 @@ namespace Dalamud.Game.Internal { private bool HandleFrameworkUpdate(IntPtr framework) { try { + dalamud.ClientState.Actors.ResetCache(); Gui.Chat.UpdateQueue(this); Network.UpdateQueue(this); } catch (Exception ex) { From 32aa32d64a32f4051443846073e84b0aa0432f39 Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 13:03:23 -0400 Subject: [PATCH 4/7] Remove ReadProcessMemory call New performance metric for the uncached case: About `55ms` --- Dalamud/Game/ClientState/Actors/ActorTable.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Dalamud/Game/ClientState/Actors/ActorTable.cs b/Dalamud/Game/ClientState/Actors/ActorTable.cs index 6369b9c6e..b430c8b40 100644 --- a/Dalamud/Game/ClientState/Actors/ActorTable.cs +++ b/Dalamud/Game/ClientState/Actors/ActorTable.cs @@ -73,14 +73,7 @@ namespace Dalamud.Game.ClientState.Actors { private Actor ReadActorFromMemory(IntPtr offset) { try { - // FIXME: hack workaround for trying to access the player on logout, after the main object has been deleted - if (!ReadProcessMemory(Process.GetCurrentProcess().Handle, offset, actorMem, actorMemSize, out _)) - { - Log.Debug("ActorTable - ReadProcessMemory failed: likely player deletion during logout"); - return null; - } - - var actorStruct = Marshal.PtrToStructure(actorMem); + var actorStruct = Marshal.PtrToStructure(offset); switch (actorStruct.ObjectKind) { case ObjectKind.Player: return new PlayerCharacter(offset, actorStruct, this.dalamud); From 2e86509a972d16c8e7a8994b3482c4d880746128 Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 13:12:23 -0400 Subject: [PATCH 5/7] Revert Framework.cs change --- Dalamud/Game/Internal/Framework.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Dalamud/Game/Internal/Framework.cs b/Dalamud/Game/Internal/Framework.cs index cdba70a8b..12f68264f 100644 --- a/Dalamud/Game/Internal/Framework.cs +++ b/Dalamud/Game/Internal/Framework.cs @@ -104,7 +104,6 @@ namespace Dalamud.Game.Internal { private bool HandleFrameworkUpdate(IntPtr framework) { try { - dalamud.ClientState.Actors.ResetCache(); Gui.Chat.UpdateQueue(this); Network.UpdateQueue(this); } catch (Exception ex) { From c50fbfab5ec84fc524bfd66ce259c783a9e06bd5 Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 13:15:23 -0400 Subject: [PATCH 6/7] Subcribe to Framework's Update event from --- Dalamud/Game/ClientState/Actors/ActorTable.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Dalamud/Game/ClientState/Actors/ActorTable.cs b/Dalamud/Game/ClientState/Actors/ActorTable.cs index b430c8b40..2f5a2224e 100644 --- a/Dalamud/Game/ClientState/Actors/ActorTable.cs +++ b/Dalamud/Game/ClientState/Actors/ActorTable.cs @@ -57,9 +57,15 @@ namespace Dalamud.Game.ClientState.Actors { Address = addressResolver; this.dalamud = dalamud; + dalamud.Framework.OnUpdateEvent += Framework_OnUpdateEvent; + Log.Verbose("Actor table address {ActorTable}", Address.ActorTable); } + private void Framework_OnUpdateEvent(Internal.Framework framework) { + this.ResetCache(); + } + /// /// Get an actor at the specified spawn index. /// @@ -140,6 +146,7 @@ namespace Dalamud.Game.ClientState.Actors { private void Dispose(bool disposing) { if (disposed) return; + this.dalamud.Framework.OnUpdateEvent -= Framework_OnUpdateEvent; Marshal.FreeHGlobal(actorMem); disposed = true; } From 84171d9d2416d9f8485fbff32f6fdba2f84388fc Mon Sep 17 00:00:00 2001 From: AzureGem Date: Sun, 19 Jul 2020 14:11:21 -0400 Subject: [PATCH 7/7] Revert Framework.cs changes I missed `Framework.dalamud` --- Dalamud/Game/Internal/Framework.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/Dalamud/Game/Internal/Framework.cs b/Dalamud/Game/Internal/Framework.cs index 12f68264f..e65ead3a0 100644 --- a/Dalamud/Game/Internal/Framework.cs +++ b/Dalamud/Game/Internal/Framework.cs @@ -23,7 +23,6 @@ namespace Dalamud.Game.Internal { public event OnUpdateDelegate OnUpdateEvent; private Hook updateHook; - private Dalamud dalamud; /// @@ -50,8 +49,6 @@ namespace Dalamud.Game.Internal { #endregion public Framework(SigScanner scanner, Dalamud dalamud) { - this.dalamud = dalamud; - Address = new FrameworkAddressResolver(); Address.Setup(scanner);