From 68389f3c2d6d603b012df8961799dc48b425d6ca Mon Sep 17 00:00:00 2001 From: raegan Date: Sun, 16 Jun 2024 17:46:10 +0200 Subject: [PATCH] List pool usage extended to other valiable places --- VContainer/Assets/Tests/ListPoolTest.cs | 66 +++++++++++++++++++ VContainer/Assets/Tests/ListPoolTest.cs.meta | 11 ++++ .../CollectionInstanceProvider.cs | 39 +++++++---- .../ContainerLocalInstanceProvider.cs | 7 +- .../ListPool.cs} | 31 +++++---- .../ListPool.cs.meta} | 0 .../FindComponentProvider.cs | 2 +- .../Unity/LifetimeScope.AwakeScheduler.cs | 49 +++++++------- .../VContainer/Runtime/Unity/LifetimeScope.cs | 3 +- .../Unity/ObjectResolverUnityExtensions.cs | 3 +- 10 files changed, 156 insertions(+), 55 deletions(-) create mode 100644 VContainer/Assets/Tests/ListPoolTest.cs create mode 100644 VContainer/Assets/Tests/ListPoolTest.cs.meta rename VContainer/Assets/VContainer/Runtime/{Unity/UnityEngineObjectListBuffer.cs => Internal/ListPool.cs} (65%) rename VContainer/Assets/VContainer/Runtime/{Unity/UnityEngineObjectListBuffer.cs.meta => Internal/ListPool.cs.meta} (100%) diff --git a/VContainer/Assets/Tests/ListPoolTest.cs b/VContainer/Assets/Tests/ListPoolTest.cs new file mode 100644 index 00000000..970c5a52 --- /dev/null +++ b/VContainer/Assets/Tests/ListPoolTest.cs @@ -0,0 +1,66 @@ +using NUnit.Framework; +using System.Collections.Generic; +using VContainer.Internal; + +namespace VContainer.Tests +{ + [TestFixture] + public class ListPoolTests + { + [Test] + public void Get_ShouldReturnNewList_WhenPoolIsEmpty() + { + var buffer = ListPool.Get(); + + Assert.IsNotNull(buffer); + Assert.AreEqual(0, buffer.Count); + } + + [Test] + public void Get_ShouldReturnListFromPool_WhenPoolIsNotEmpty() + { + var initialBuffer = ListPool.Get(); + ListPool.Release(initialBuffer); + + var buffer = ListPool.Get(); + Assert.AreSame(initialBuffer, buffer); + } + + [Test] + public void Release_ShouldClearAndReturnListToPool() + { + var buffer = ListPool.Get(); + buffer.Add(1); + + ListPool.Release(buffer); + Assert.AreEqual(0, buffer.Count); + } + + [Test] + public void BufferScope_ShouldReleaseBuffer_WhenDisposed() + { + List buffer; + using (ListPool.Get(out buffer)) + { + buffer.Add(1); + } + + Assert.AreEqual(0, buffer.Count); + } + + [Test] + public void Get_And_Release_MultipleBuffers() + { + var buffer1 = ListPool.Get(); + var buffer2 = ListPool.Get(); + + ListPool.Release(buffer1); + ListPool.Release(buffer2); + + var bufferFromPool1 = ListPool.Get(); + var bufferFromPool2 = ListPool.Get(); + + Assert.AreNotSame(bufferFromPool1, bufferFromPool2); + } + } +} diff --git a/VContainer/Assets/Tests/ListPoolTest.cs.meta b/VContainer/Assets/Tests/ListPoolTest.cs.meta new file mode 100644 index 00000000..a0819806 --- /dev/null +++ b/VContainer/Assets/Tests/ListPoolTest.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7ff27995f2ef7714c800c9a9cec2e769 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/CollectionInstanceProvider.cs b/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/CollectionInstanceProvider.cs index 6ee8d806..b90cf9f6 100644 --- a/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/CollectionInstanceProvider.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/CollectionInstanceProvider.cs @@ -57,31 +57,36 @@ public object SpawnInstance(IObjectResolver resolver) { if (resolver is IScopedObjectResolver scope) { - var entirelyRegistrations = CollectFromParentScopes(scope); - return SpawnInstance(resolver, entirelyRegistrations); + using (ListPool.Get(out var entirelyRegistrations)) + { + CollectFromParentScopes(scope, entirelyRegistrations); + return SpawnInstance(resolver, entirelyRegistrations);; + } } return SpawnInstance(resolver, registrations); } internal object SpawnInstance( IObjectResolver resolver, - IReadOnlyList registrations) + IReadOnlyList entirelyRegistrations) { - var array = Array.CreateInstance(ElementType, registrations.Count); - for (var i = 0; i < registrations.Count; i++) + var array = Array.CreateInstance(ElementType, entirelyRegistrations.Count); + for (var i = 0; i < entirelyRegistrations.Count; i++) { - array.SetValue(resolver.Resolve(registrations[i]), i); + array.SetValue(resolver.Resolve(entirelyRegistrations[i]), i); } return array; } - internal List CollectFromParentScopes( + internal void CollectFromParentScopes( IScopedObjectResolver scope, + List registrationsBuffer, bool localScopeOnly = false) { if (scope.Parent == null) { - return registrations; + registrationsBuffer.AddRange(registrations); + return; } var finderType = InterfaceTypes[0]; @@ -93,11 +98,8 @@ internal List CollectFromParentScopes( if (scope.TryGetRegistration(finderType, out var registration) && registration.Provider is CollectionInstanceProvider parentCollection) { - if (mergedRegistrations == null) - { - // TODO: object pooling - mergedRegistrations = new List(registrations); - } + mergedRegistrations ??= ListPool.Get(); + mergedRegistrations.AddRange(registrations); if (localScopeOnly) { @@ -116,7 +118,16 @@ internal List CollectFromParentScopes( } scope = scope.Parent; } - return mergedRegistrations ?? registrations; + + if (mergedRegistrations == null) + { + registrationsBuffer.AddRange(registrations); + } + else + { + registrationsBuffer.AddRange(mergedRegistrations); + ListPool.Release(mergedRegistrations); + } } } } diff --git a/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/ContainerLocalInstanceProvider.cs b/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/ContainerLocalInstanceProvider.cs index 54b122bf..d81542fd 100644 --- a/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/ContainerLocalInstanceProvider.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/InstanceProviders/ContainerLocalInstanceProvider.cs @@ -20,8 +20,11 @@ public object SpawnInstance(IObjectResolver resolver) if (resolver is ScopedContainer scope && valueRegistration.Provider is CollectionInstanceProvider collectionProvider) { - var entirelyRegistrations = collectionProvider.CollectFromParentScopes(scope, localScopeOnly: true); - value = collectionProvider.SpawnInstance(resolver, entirelyRegistrations); + using (ListPool.Get(out var entirelyRegistrations)) + { + collectionProvider.CollectFromParentScopes(scope, entirelyRegistrations, localScopeOnly: true); + value = collectionProvider.SpawnInstance(resolver, entirelyRegistrations); + } } else { diff --git a/VContainer/Assets/VContainer/Runtime/Unity/UnityEngineObjectListBuffer.cs b/VContainer/Assets/VContainer/Runtime/Internal/ListPool.cs similarity index 65% rename from VContainer/Assets/VContainer/Runtime/Unity/UnityEngineObjectListBuffer.cs rename to VContainer/Assets/VContainer/Runtime/Internal/ListPool.cs index 31c8b16d..344a9ec0 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/UnityEngineObjectListBuffer.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/ListPool.cs @@ -1,19 +1,18 @@ using System; using System.Collections.Generic; -namespace VContainer.Unity +namespace VContainer.Internal { - static class UnityEngineObjectListBuffer where T : UnityEngine.Object + internal static class ListPool { const int DefaultCapacity = 32; - [ThreadStatic] - private static Stack> _pool = new Stack>(4); + private static readonly Stack> _pool = new Stack>(4); /// /// BufferScope supports releasing a buffer with using clause. /// - public struct BufferScope : IDisposable + internal readonly struct BufferScope : IDisposable { private readonly List _buffer; @@ -32,14 +31,17 @@ public void Dispose() /// Get a buffer from the pool. /// /// - public static List Get() + internal static List Get() { - if (_pool.Count == 0) + lock (_pool) { - return new List(DefaultCapacity); - } + if (_pool.Count == 0) + { + return new List(DefaultCapacity); + } - return _pool.Pop(); + return _pool.Pop(); + } } /// @@ -47,7 +49,7 @@ public static List Get() /// /// /// - public static BufferScope Get(out List buffer) + internal static BufferScope Get(out List buffer) { buffer = Get(); return new BufferScope(buffer); @@ -57,10 +59,13 @@ public static BufferScope Get(out List buffer) /// Declare a buffer won't be used anymore and put it back to the pool. /// /// - public static void Release(List buffer) + internal static void Release(List buffer) { buffer.Clear(); - _pool.Push(buffer); + lock (_pool) + { + _pool.Push(buffer); + } } } } \ No newline at end of file diff --git a/VContainer/Assets/VContainer/Runtime/Unity/UnityEngineObjectListBuffer.cs.meta b/VContainer/Assets/VContainer/Runtime/Internal/ListPool.cs.meta similarity index 100% rename from VContainer/Assets/VContainer/Runtime/Unity/UnityEngineObjectListBuffer.cs.meta rename to VContainer/Assets/VContainer/Runtime/Internal/ListPool.cs.meta diff --git a/VContainer/Assets/VContainer/Runtime/Unity/InstanceProviders/FindComponentProvider.cs b/VContainer/Assets/VContainer/Runtime/Unity/InstanceProviders/FindComponentProvider.cs index 364a5fb1..bafabe9b 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/InstanceProviders/FindComponentProvider.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/InstanceProviders/FindComponentProvider.cs @@ -40,7 +40,7 @@ public object SpawnInstance(IObjectResolver resolver) } else if (scene.IsValid()) { - using (UnityEngineObjectListBuffer.Get(out var gameObjectBuffer)) + using (ListPool.Get(out var gameObjectBuffer)) { scene.GetRootGameObjects(gameObjectBuffer); foreach (var gameObject in gameObjectBuffer) diff --git a/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.AwakeScheduler.cs b/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.AwakeScheduler.cs index 4dff7fbe..f9f9b509 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.AwakeScheduler.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.AwakeScheduler.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using UnityEngine; using UnityEngine.SceneManagement; +using VContainer.Internal; namespace VContainer.Unity { @@ -45,22 +46,23 @@ static void AwakeWaitingChildren(LifetimeScope awakenParent) { if (WaitingList.Count <= 0) return; - var buf = new List(); - - for (var i = WaitingList.Count - 1; i >= 0; i--) + using (ListPool.Get(out var buffer)) { - var waitingScope = WaitingList[i]; - if (waitingScope.parentReference.Type == awakenParent.GetType()) + for (var i = WaitingList.Count - 1; i >= 0; i--) { - waitingScope.parentReference.Object = awakenParent; - WaitingList.RemoveAt(i); - buf.Add(waitingScope); + var waitingScope = WaitingList[i]; + if (waitingScope.parentReference.Type == awakenParent.GetType()) + { + waitingScope.parentReference.Object = awakenParent; + WaitingList.RemoveAt(i); + buffer.Add(waitingScope); + } } - } - foreach (var waitingScope in buf) - { - waitingScope.Awake(); + foreach (var waitingScope in buffer) + { + waitingScope.Awake(); + } } } @@ -69,21 +71,22 @@ static void OnSceneLoaded(Scene scene, LoadSceneMode mode) if (WaitingList.Count <= 0) return; - var buf = new List(); - - for (var i = WaitingList.Count - 1; i >= 0; i--) + using (ListPool.Get(out var buffer)) { - var waitingScope = WaitingList[i]; - if (waitingScope.gameObject.scene == scene) + for (var i = WaitingList.Count - 1; i >= 0; i--) { - WaitingList.RemoveAt(i); - buf.Add(waitingScope); + var waitingScope = WaitingList[i]; + if (waitingScope.gameObject.scene == scene) + { + WaitingList.RemoveAt(i); + buffer.Add(waitingScope); + } } - } - foreach (var waitingScope in buf) - { - waitingScope.Awake(); // Re-throw if parent not found + foreach (var waitingScope in buffer) + { + waitingScope.Awake(); // Re-throw if parent not found + } } } } diff --git a/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.cs b/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.cs index 3f64aa2d..e5637e8b 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/LifetimeScope.cs @@ -3,6 +3,7 @@ using UnityEngine; using UnityEngine.SceneManagement; using VContainer.Diagnostics; +using VContainer.Internal; namespace VContainer.Unity { @@ -101,7 +102,7 @@ public static ExtraInstallationScope Enqueue(IInstaller installer) static LifetimeScope Find(Type type, Scene scene) { - using (UnityEngineObjectListBuffer.Get(out var buffer)) + using (ListPool.Get(out var buffer)) { scene.GetRootGameObjects(buffer); foreach (var gameObject in buffer) diff --git a/VContainer/Assets/VContainer/Runtime/Unity/ObjectResolverUnityExtensions.cs b/VContainer/Assets/VContainer/Runtime/Unity/ObjectResolverUnityExtensions.cs index fcff370d..15af3d90 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/ObjectResolverUnityExtensions.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/ObjectResolverUnityExtensions.cs @@ -1,4 +1,5 @@ using UnityEngine; +using VContainer.Internal; namespace VContainer.Unity { @@ -10,7 +11,7 @@ void InjectGameObjectRecursive(GameObject current) { if (current == null) return; - using (UnityEngineObjectListBuffer.Get(out var buffer)) + using (ListPool.Get(out var buffer)) { buffer.Clear(); current.GetComponents(buffer);