From 88a8d457989bab22b06295192382a0eccfce2eab Mon Sep 17 00:00:00 2001 From: srkizer Date: Fri, 8 Mar 2024 10:47:11 +0900 Subject: [PATCH] Accommodate nested AddonLifecycle event calls (#1698) * Accommodate nested AddonLifecycle event calls The game is free to call event handlers of another addon from one addon, but the previous code was written under the assumption that only one function may be called at a time. This changes the recycled addon args into pooled args. * Always clear addon name cache --- Dalamud/Dalamud.csproj | 4 - .../Game/Addon/AddonLifecyclePooledArgs.cs | 107 ++++++++++++++++++ .../Game/Addon/Events/AddonEventManager.cs | 2 +- .../Lifecycle/AddonArgTypes/AddonArgs.cs | 6 +- .../Game/Addon/Lifecycle/AddonLifecycle.cs | 84 +++++++------- .../AddonLifecycleReceiveEventListener.cs | 31 +++-- 6 files changed, 165 insertions(+), 69 deletions(-) create mode 100644 Dalamud/Game/Addon/AddonLifecyclePooledArgs.cs diff --git a/Dalamud/Dalamud.csproj b/Dalamud/Dalamud.csproj index 205681cb8..7e166d8b3 100644 --- a/Dalamud/Dalamud.csproj +++ b/Dalamud/Dalamud.csproj @@ -112,10 +112,6 @@ - - - - diff --git a/Dalamud/Game/Addon/AddonLifecyclePooledArgs.cs b/Dalamud/Game/Addon/AddonLifecyclePooledArgs.cs new file mode 100644 index 000000000..14def2036 --- /dev/null +++ b/Dalamud/Game/Addon/AddonLifecyclePooledArgs.cs @@ -0,0 +1,107 @@ +using System.Runtime.CompilerServices; +using System.Threading; + +using Dalamud.Game.Addon.Lifecycle.AddonArgTypes; + +namespace Dalamud.Game.Addon; + +/// Argument pool for Addon Lifecycle services. +[ServiceManager.EarlyLoadedService] +internal sealed class AddonLifecyclePooledArgs : IServiceType +{ + private readonly AddonSetupArgs?[] addonSetupArgPool = new AddonSetupArgs?[64]; + private readonly AddonFinalizeArgs?[] addonFinalizeArgPool = new AddonFinalizeArgs?[64]; + private readonly AddonDrawArgs?[] addonDrawArgPool = new AddonDrawArgs?[64]; + private readonly AddonUpdateArgs?[] addonUpdateArgPool = new AddonUpdateArgs?[64]; + private readonly AddonRefreshArgs?[] addonRefreshArgPool = new AddonRefreshArgs?[64]; + private readonly AddonRequestedUpdateArgs?[] addonRequestedUpdateArgPool = new AddonRequestedUpdateArgs?[64]; + private readonly AddonReceiveEventArgs?[] addonReceiveEventArgPool = new AddonReceiveEventArgs?[64]; + + [ServiceManager.ServiceConstructor] + private AddonLifecyclePooledArgs() + { + } + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonSetupArgs arg) => new(out arg, this.addonSetupArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonFinalizeArgs arg) => new(out arg, this.addonFinalizeArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonDrawArgs arg) => new(out arg, this.addonDrawArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonUpdateArgs arg) => new(out arg, this.addonUpdateArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonRefreshArgs arg) => new(out arg, this.addonRefreshArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonRequestedUpdateArgs arg) => + new(out arg, this.addonRequestedUpdateArgPool); + + /// Rents an instance of an argument. + /// The rented instance. + /// The returner. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public PooledEntry Rent(out AddonReceiveEventArgs arg) => + new(out arg, this.addonReceiveEventArgPool); + + /// Returns the object to the pool on dispose. + /// The type. + public readonly ref struct PooledEntry + where T : AddonArgs, new() + { + private readonly Span pool; + private readonly T obj; + + /// Initializes a new instance of the struct. + /// An instance of the argument. + /// The pool to rent from and return to. + public PooledEntry(out T arg, Span pool) + { + this.pool = pool; + foreach (ref var item in pool) + { + if (Interlocked.Exchange(ref item, null) is { } v) + { + this.obj = arg = v; + return; + } + } + + this.obj = arg = new(); + } + + /// Returns the item to the pool. + public void Dispose() + { + var tmp = this.obj; + foreach (ref var item in this.pool) + { + if (Interlocked.Exchange(ref item, tmp) is not { } tmp2) + return; + tmp = tmp2; + } + } + } +} diff --git a/Dalamud/Game/Addon/Events/AddonEventManager.cs b/Dalamud/Game/Addon/Events/AddonEventManager.cs index 4231b0d09..8ee09bed8 100644 --- a/Dalamud/Game/Addon/Events/AddonEventManager.cs +++ b/Dalamud/Game/Addon/Events/AddonEventManager.cs @@ -18,7 +18,7 @@ namespace Dalamud.Game.Addon.Events; /// Service provider for addon event management. /// [InterfaceVersion("1.0")] -[ServiceManager.BlockingEarlyLoadedService] +[ServiceManager.EarlyLoadedService] internal unsafe class AddonEventManager : IDisposable, IServiceType { /// diff --git a/Dalamud/Game/Addon/Lifecycle/AddonArgTypes/AddonArgs.cs b/Dalamud/Game/Addon/Lifecycle/AddonArgTypes/AddonArgs.cs index 4ab3de5ca..1095202cc 100644 --- a/Dalamud/Game/Addon/Lifecycle/AddonArgTypes/AddonArgs.cs +++ b/Dalamud/Game/Addon/Lifecycle/AddonArgTypes/AddonArgs.cs @@ -44,10 +44,10 @@ public abstract unsafe class AddonArgs get => this.addon; set { - if (this.addon == value) - return; - this.addon = value; + + // Note: always clear addonName on updating the addon being pointed. + // Same address may point to a different addon. this.addonName = null; } } diff --git a/Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs b/Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs index beaab7fcd..37f12ce3a 100644 --- a/Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs +++ b/Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; @@ -19,7 +18,7 @@ namespace Dalamud.Game.Addon.Lifecycle; /// This class provides events for in-game addon lifecycles. /// [InterfaceVersion("1.0")] -[ServiceManager.BlockingEarlyLoadedService] +[ServiceManager.EarlyLoadedService] internal unsafe class AddonLifecycle : IDisposable, IServiceType { private static readonly ModuleLog Log = new("AddonLifecycle"); @@ -27,6 +26,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType [ServiceManager.ServiceDependency] private readonly Framework framework = Service.Get(); + [ServiceManager.ServiceDependency] + private readonly AddonLifecyclePooledArgs argsPool = Service.Get(); + private readonly nint disallowedReceiveEventAddress; private readonly AddonLifecycleAddressResolver address; @@ -38,18 +40,6 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType private readonly Hook onAddonRefreshHook; private readonly CallHook onAddonRequestedUpdateHook; - // Note: these can be sourced from ObjectPool of appropriate types instead, but since we don't import that NuGet - // package, and these events are always called from the main thread, this is fine. -#pragma warning disable CS0618 // Type or member is obsolete - // TODO: turn constructors of these internal - private readonly AddonSetupArgs recyclingSetupArgs = new(); - private readonly AddonFinalizeArgs recyclingFinalizeArgs = new(); - private readonly AddonDrawArgs recyclingDrawArgs = new(); - private readonly AddonUpdateArgs recyclingUpdateArgs = new(); - private readonly AddonRefreshArgs recyclingRefreshArgs = new(); - private readonly AddonRequestedUpdateArgs recyclingRequestedUpdateArgs = new(); -#pragma warning restore CS0618 // Type or member is obsolete - [ServiceManager.ServiceConstructor] private AddonLifecycle(TargetSigScanner sigScanner) { @@ -253,12 +243,13 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Exception in OnAddonSetup ReceiveEvent Registration."); } - this.recyclingSetupArgs.AddonInternal = (nint)addon; - this.recyclingSetupArgs.AtkValueCount = valueCount; - this.recyclingSetupArgs.AtkValues = (nint)values; - this.InvokeListenersSafely(AddonEvent.PreSetup, this.recyclingSetupArgs); - valueCount = this.recyclingSetupArgs.AtkValueCount; - values = (AtkValue*)this.recyclingSetupArgs.AtkValues; + using var returner = this.argsPool.Rent(out AddonSetupArgs arg); + arg.AddonInternal = (nint)addon; + arg.AtkValueCount = valueCount; + arg.AtkValues = (nint)values; + this.InvokeListenersSafely(AddonEvent.PreSetup, arg); + valueCount = arg.AtkValueCount; + values = (AtkValue*)arg.AtkValues; try { @@ -269,7 +260,7 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Caught exception when calling original AddonSetup. This may be a bug in the game or another plugin hooking this method."); } - this.InvokeListenersSafely(AddonEvent.PostSetup, this.recyclingSetupArgs); + this.InvokeListenersSafely(AddonEvent.PostSetup, arg); } private void OnAddonFinalize(AtkUnitManager* unitManager, AtkUnitBase** atkUnitBase) @@ -284,8 +275,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Exception in OnAddonFinalize ReceiveEvent Removal."); } - this.recyclingFinalizeArgs.AddonInternal = (nint)atkUnitBase[0]; - this.InvokeListenersSafely(AddonEvent.PreFinalize, this.recyclingFinalizeArgs); + using var returner = this.argsPool.Rent(out AddonFinalizeArgs arg); + arg.AddonInternal = (nint)atkUnitBase[0]; + this.InvokeListenersSafely(AddonEvent.PreFinalize, arg); try { @@ -299,8 +291,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType private void OnAddonDraw(AtkUnitBase* addon) { - this.recyclingDrawArgs.AddonInternal = (nint)addon; - this.InvokeListenersSafely(AddonEvent.PreDraw, this.recyclingDrawArgs); + using var returner = this.argsPool.Rent(out AddonDrawArgs arg); + arg.AddonInternal = (nint)addon; + this.InvokeListenersSafely(AddonEvent.PreDraw, arg); try { @@ -311,14 +304,15 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Caught exception when calling original AddonDraw. This may be a bug in the game or another plugin hooking this method."); } - this.InvokeListenersSafely(AddonEvent.PostDraw, this.recyclingDrawArgs); + this.InvokeListenersSafely(AddonEvent.PostDraw, arg); } private void OnAddonUpdate(AtkUnitBase* addon, float delta) { - this.recyclingUpdateArgs.AddonInternal = (nint)addon; - this.recyclingUpdateArgs.TimeDeltaInternal = delta; - this.InvokeListenersSafely(AddonEvent.PreUpdate, this.recyclingUpdateArgs); + using var returner = this.argsPool.Rent(out AddonUpdateArgs arg); + arg.AddonInternal = (nint)addon; + arg.TimeDeltaInternal = delta; + this.InvokeListenersSafely(AddonEvent.PreUpdate, arg); try { @@ -329,19 +323,20 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Caught exception when calling original AddonUpdate. This may be a bug in the game or another plugin hooking this method."); } - this.InvokeListenersSafely(AddonEvent.PostUpdate, this.recyclingUpdateArgs); + this.InvokeListenersSafely(AddonEvent.PostUpdate, arg); } private byte OnAddonRefresh(AtkUnitManager* atkUnitManager, AtkUnitBase* addon, uint valueCount, AtkValue* values) { byte result = 0; - this.recyclingRefreshArgs.AddonInternal = (nint)addon; - this.recyclingRefreshArgs.AtkValueCount = valueCount; - this.recyclingRefreshArgs.AtkValues = (nint)values; - this.InvokeListenersSafely(AddonEvent.PreRefresh, this.recyclingRefreshArgs); - valueCount = this.recyclingRefreshArgs.AtkValueCount; - values = (AtkValue*)this.recyclingRefreshArgs.AtkValues; + using var returner = this.argsPool.Rent(out AddonRefreshArgs arg); + arg.AddonInternal = (nint)addon; + arg.AtkValueCount = valueCount; + arg.AtkValues = (nint)values; + this.InvokeListenersSafely(AddonEvent.PreRefresh, arg); + valueCount = arg.AtkValueCount; + values = (AtkValue*)arg.AtkValues; try { @@ -352,18 +347,19 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Caught exception when calling original AddonRefresh. This may be a bug in the game or another plugin hooking this method."); } - this.InvokeListenersSafely(AddonEvent.PostRefresh, this.recyclingRefreshArgs); + this.InvokeListenersSafely(AddonEvent.PostRefresh, arg); return result; } private void OnRequestedUpdate(AtkUnitBase* addon, NumberArrayData** numberArrayData, StringArrayData** stringArrayData) { - this.recyclingRequestedUpdateArgs.AddonInternal = (nint)addon; - this.recyclingRequestedUpdateArgs.NumberArrayData = (nint)numberArrayData; - this.recyclingRequestedUpdateArgs.StringArrayData = (nint)stringArrayData; - this.InvokeListenersSafely(AddonEvent.PreRequestedUpdate, this.recyclingRequestedUpdateArgs); - numberArrayData = (NumberArrayData**)this.recyclingRequestedUpdateArgs.NumberArrayData; - stringArrayData = (StringArrayData**)this.recyclingRequestedUpdateArgs.StringArrayData; + using var returner = this.argsPool.Rent(out AddonRequestedUpdateArgs arg); + arg.AddonInternal = (nint)addon; + arg.NumberArrayData = (nint)numberArrayData; + arg.StringArrayData = (nint)stringArrayData; + this.InvokeListenersSafely(AddonEvent.PreRequestedUpdate, arg); + numberArrayData = (NumberArrayData**)arg.NumberArrayData; + stringArrayData = (StringArrayData**)arg.StringArrayData; try { @@ -374,7 +370,7 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType Log.Error(e, "Caught exception when calling original AddonRequestedUpdate. This may be a bug in the game or another plugin hooking this method."); } - this.InvokeListenersSafely(AddonEvent.PostRequestedUpdate, this.recyclingRequestedUpdateArgs); + this.InvokeListenersSafely(AddonEvent.PostRequestedUpdate, arg); } } diff --git a/Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs b/Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs index 43aa71661..fd3b5d79d 100644 --- a/Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs +++ b/Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs @@ -16,12 +16,8 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable { private static readonly ModuleLog Log = new("AddonLifecycle"); - // Note: these can be sourced from ObjectPool of appropriate types instead, but since we don't import that NuGet - // package, and these events are always called from the main thread, this is fine. -#pragma warning disable CS0618 // Type or member is obsolete - // TODO: turn constructors of these internal - private readonly AddonReceiveEventArgs recyclingReceiveEventArgs = new(); -#pragma warning restore CS0618 // Type or member is obsolete + [ServiceManager.ServiceDependency] + private readonly AddonLifecyclePooledArgs argsPool = Service.Get(); /// /// Initializes a new instance of the class. @@ -82,16 +78,17 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable return; } - this.recyclingReceiveEventArgs.AddonInternal = (nint)addon; - this.recyclingReceiveEventArgs.AtkEventType = (byte)eventType; - this.recyclingReceiveEventArgs.EventParam = eventParam; - this.recyclingReceiveEventArgs.AtkEvent = (IntPtr)atkEvent; - this.recyclingReceiveEventArgs.Data = data; - this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PreReceiveEvent, this.recyclingReceiveEventArgs); - eventType = (AtkEventType)this.recyclingReceiveEventArgs.AtkEventType; - eventParam = this.recyclingReceiveEventArgs.EventParam; - atkEvent = (AtkEvent*)this.recyclingReceiveEventArgs.AtkEvent; - data = this.recyclingReceiveEventArgs.Data; + using var returner = this.argsPool.Rent(out AddonReceiveEventArgs arg); + arg.AddonInternal = (nint)addon; + arg.AtkEventType = (byte)eventType; + arg.EventParam = eventParam; + arg.AtkEvent = (IntPtr)atkEvent; + arg.Data = data; + this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PreReceiveEvent, arg); + eventType = (AtkEventType)arg.AtkEventType; + eventParam = arg.EventParam; + atkEvent = (AtkEvent*)arg.AtkEvent; + data = arg.Data; try { @@ -102,6 +99,6 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable Log.Error(e, "Caught exception when calling original AddonReceiveEvent. This may be a bug in the game or another plugin hooking this method."); } - this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PostReceiveEvent, this.recyclingReceiveEventArgs); + this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PostReceiveEvent, arg); } }