From cc2ff05a5f4856c2d7a791279cd7a7574d06fe98 Mon Sep 17 00:00:00 2001 From: Juan Ignacio Molteni Date: Thu, 24 Oct 2024 12:07:22 -0300 Subject: [PATCH] chore: UnloadStrategy abstract class (#2508) --- .../Global/ResourceUnloadingPlugin.cs | 1 - .../Tests/ReleaseMemorySystemShould.cs | 80 +++++++++++-------- .../UnloadStrategies/IUnloadStrategy.cs | 34 -------- .../ReduceLoadingRadiusUnloadStrategy.cs | 9 ++- .../StandardUnloadStrategy.cs | 18 ++++- .../UnloadStrategies/UnloadStrategyBase.cs | 31 +++++++ ...egy.cs.meta => UnloadStrategyBase.cs.meta} | 0 .../UnloadStrategies/UnloadStrategyHandler.cs | 31 ++++--- .../UnloadUnusedAssetUnloadStrategy.cs | 15 ++-- 9 files changed, 119 insertions(+), 100 deletions(-) delete mode 100644 Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs create mode 100644 Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs rename Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/{IUnloadStrategy.cs.meta => UnloadStrategyBase.cs.meta} (100%) diff --git a/Explorer/Assets/DCL/PluginSystem/Global/ResourceUnloadingPlugin.cs b/Explorer/Assets/DCL/PluginSystem/Global/ResourceUnloadingPlugin.cs index 09a7146375..82d53abe27 100644 --- a/Explorer/Assets/DCL/PluginSystem/Global/ResourceUnloadingPlugin.cs +++ b/Explorer/Assets/DCL/PluginSystem/Global/ResourceUnloadingPlugin.cs @@ -1,5 +1,4 @@ using Arch.SystemGroups; -using Cysharp.Threading.Tasks; using DCL.Optimization.PerformanceBudgeting; using DCL.ResourcesUnloading; using DCL.ResourcesUnloading.UnloadStrategies; diff --git a/Explorer/Assets/DCL/ResourcesUnloading/Tests/ReleaseMemorySystemShould.cs b/Explorer/Assets/DCL/ResourcesUnloading/Tests/ReleaseMemorySystemShould.cs index a523ad7e8a..1b6fbda90f 100644 --- a/Explorer/Assets/DCL/ResourcesUnloading/Tests/ReleaseMemorySystemShould.cs +++ b/Explorer/Assets/DCL/ResourcesUnloading/Tests/ReleaseMemorySystemShould.cs @@ -10,6 +10,20 @@ namespace DCL.ResourcesUnloading.Tests { public class ReleaseMemorySystemShould : UnitySystemTestBase { + public class MockUnloadStrategy : UnloadStrategyBase + { + public int strategyRunCount; + + public override void RunStrategy() + { + strategyRunCount++; + } + + public MockUnloadStrategy(int failureThreshold) : base(failureThreshold) + { + } + } + private ReleaseMemorySystem releaseMemorySystem; @@ -17,10 +31,10 @@ public class ReleaseMemorySystemShould : UnitySystemTestBase(); cacheCleaner = Substitute.For(); - standardStrategy = Substitute.For(); - aggresiveStrategy = Substitute.For(); + standardStrategy = new MockUnloadStrategy(1); + aggresiveStrategy = new MockUnloadStrategy(1); unloadStrategies = new[] { @@ -52,15 +66,12 @@ public void SetUp() [TestCase(MemoryUsageStatus.FULL, 1)] public void UnloadCacheWhenMemoryUsageIsNotNormal(MemoryUsageStatus memoryUsageStatus, int callsAmount) { - // Arrange - memoryBudgetProvider.GetMemoryUsageStatus().Returns(memoryUsageStatus); - standardStrategy.FailedOverThreshold().Returns(true); - // Act + memoryBudgetProvider.GetMemoryUsageStatus().Returns(memoryUsageStatus); releaseMemorySystem.Update(0); // Assert - standardStrategy.Received(callsAmount).TryUnload(cacheCleaner); + Assert.AreEqual(callsAmount, standardStrategy.strategyRunCount); } [Test] @@ -68,22 +79,19 @@ public void ResetUnloadStrategyIndexWhenMemoryUsageIsNormal() { // Arrange memoryBudgetProvider.GetMemoryUsageStatus().Returns(MemoryUsageStatus.WARNING); - standardStrategy.FailedOverThreshold().Returns(true); // Act releaseMemorySystem.Update(0); // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 1); - standardStrategy.Received(1).TryUnload(cacheCleaner); + Assert.AreEqual(1, standardStrategy.strategyRunCount); // Act releaseMemorySystem.Update(0); // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 1); - standardStrategy.Received(1).TryUnload(cacheCleaner); - aggresiveStrategy.Received(1).TryUnload(cacheCleaner); + Assert.AreEqual(2, standardStrategy.strategyRunCount); + Assert.AreEqual(1, aggresiveStrategy.strategyRunCount); // Act @@ -91,9 +99,9 @@ public void ResetUnloadStrategyIndexWhenMemoryUsageIsNormal() releaseMemorySystem.Update(0); // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 0); - standardStrategy.Received(1).ResetStrategy(); - aggresiveStrategy.Received(1).ResetStrategy(); + Assert.AreEqual(0, standardStrategy.currentFailureCount); + Assert.AreEqual(0, aggresiveStrategy.currentFailureCount); + Assert.IsFalse(standardStrategy.FaillingOverThreshold()); } [Test] @@ -101,30 +109,36 @@ public void IncreaseTierAggresiveness() { // Arrange memoryBudgetProvider.GetMemoryUsageStatus().Returns(MemoryUsageStatus.WARNING); - standardStrategy.FailedOverThreshold().Returns(true); - // Act releaseMemorySystem.Update(0); // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 1); - standardStrategy.Received(1).TryUnload(cacheCleaner); + Assert.AreEqual(1, standardStrategy.strategyRunCount); // Act releaseMemorySystem.Update(0); - - // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 1); - standardStrategy.Received(1).TryUnload(cacheCleaner); - aggresiveStrategy.Received(1).TryUnload(cacheCleaner); + // Assert + Assert.AreEqual(2, standardStrategy.strategyRunCount); + Assert.AreEqual(1, aggresiveStrategy.strategyRunCount); + } + + [Test] + public void SkipAggressiveStrategyIfPreviousDidNotFail() + { + // Arrange + memoryBudgetProvider.GetMemoryUsageStatus().Returns(MemoryUsageStatus.WARNING); + standardStrategy.failureThreshold = 5; + // Act - releaseMemorySystem.Update(0); - + + for (var i = 0; i < 5; i++) + releaseMemorySystem.Update(0); + + // Assert - Assert.AreEqual(unloadStrategyHandler.currentUnloadStrategy, 1); - standardStrategy.Received(1).TryUnload(cacheCleaner); - aggresiveStrategy.Received(2).TryUnload(cacheCleaner); + Assert.AreEqual(5, standardStrategy.strategyRunCount); + Assert.AreEqual(0, aggresiveStrategy.strategyRunCount); } diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs deleted file mode 100644 index abebc7464e..0000000000 --- a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs +++ /dev/null @@ -1,34 +0,0 @@ -namespace DCL.ResourcesUnloading.UnloadStrategies -{ - - public interface IUnloadStrategy - { - void ResetStrategy(); - void TryUnload(ICacheCleaner cacheCleaner); - bool FailedOverThreshold(); - } - - public abstract class UnloadStrategy : IUnloadStrategy - { - - private int currentFailureCount; - private readonly int FAILURE_THRESHOLD = 250; - - public virtual void ResetStrategy() - { - currentFailureCount = 0; - } - - public bool FailedOverThreshold() - { - return currentFailureCount > FAILURE_THRESHOLD; - } - - public virtual void TryUnload(ICacheCleaner cacheCleaner) - { - cacheCleaner.UnloadCache(); - cacheCleaner.UpdateProfilingCounters(); - currentFailureCount++; - } - } -} \ No newline at end of file diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/ReduceLoadingRadiusUnloadStrategy.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/ReduceLoadingRadiusUnloadStrategy.cs index da8855151c..030e4c8ade 100644 --- a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/ReduceLoadingRadiusUnloadStrategy.cs +++ b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/ReduceLoadingRadiusUnloadStrategy.cs @@ -1,23 +1,24 @@ using System; using ECS.Prioritization; +using UnityEngine; namespace DCL.ResourcesUnloading.UnloadStrategies { - public class ReduceLoadingRadiusUnloadStrategy : UnloadStrategy + public class ReduceLoadingRadiusUnloadStrategy : UnloadStrategyBase { private readonly IRealmPartitionSettings realmPartitionSettings; - public ReduceLoadingRadiusUnloadStrategy(IRealmPartitionSettings realmPartitionSettings) + public ReduceLoadingRadiusUnloadStrategy(int failureThreshold, IRealmPartitionSettings realmPartitionSettings) : + base(failureThreshold) { this.realmPartitionSettings = realmPartitionSettings; } - public override void TryUnload(ICacheCleaner cacheCleaner) + public override void RunStrategy() { //Forces MaxLoadingDistanceInParcels to the minimum value //TODO (Juani): A message warning that the distance has been silently modified realmPartitionSettings.MaxLoadingDistanceInParcels = realmPartitionSettings.MinLoadingDistanceInParcels; - base.TryUnload(cacheCleaner); } } diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/StandardUnloadStrategy.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/StandardUnloadStrategy.cs index bc9deaaced..1011c7b44a 100644 --- a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/StandardUnloadStrategy.cs +++ b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/StandardUnloadStrategy.cs @@ -1,8 +1,20 @@ +using UnityEngine; + namespace DCL.ResourcesUnloading.UnloadStrategies { - public class StandardUnloadStrategy : UnloadStrategy + public class StandardUnloadStrategy : UnloadStrategyBase { - - + private readonly ICacheCleaner cacheCleaner; + + public override void RunStrategy() + { + cacheCleaner.UnloadCache(); + cacheCleaner.UpdateProfilingCounters(); + } + + public StandardUnloadStrategy(int failureThreshold, ICacheCleaner cacheCleaner) : base(failureThreshold) + { + this.cacheCleaner = cacheCleaner; + } } } \ No newline at end of file diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs new file mode 100644 index 0000000000..2155baa89b --- /dev/null +++ b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs @@ -0,0 +1,31 @@ +namespace DCL.ResourcesUnloading.UnloadStrategies +{ + public abstract class UnloadStrategyBase + { + internal int currentFailureCount; + internal int failureThreshold; + + protected UnloadStrategyBase(int failureThreshold) + { + this.failureThreshold = failureThreshold; + } + + public virtual void ResetStrategy() + { + currentFailureCount = 0; + } + + public bool FaillingOverThreshold() + { + return currentFailureCount >= failureThreshold; + } + + public abstract void RunStrategy(); + + public void TryUnload() + { + RunStrategy(); + currentFailureCount++; + } + } +} \ No newline at end of file diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs.meta b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs.meta similarity index 100% rename from Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/IUnloadStrategy.cs.meta rename to Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyBase.cs.meta diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyHandler.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyHandler.cs index c297780046..59e5ad728e 100644 --- a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyHandler.cs +++ b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadStrategyHandler.cs @@ -5,43 +5,40 @@ namespace DCL.ResourcesUnloading.UnloadStrategies { public class UnloadStrategyHandler { - internal IUnloadStrategy[] unloadStrategies; - internal int currentUnloadStrategy; + internal UnloadStrategyBase[] unloadStrategies; private readonly ICacheCleaner cacheCleaner; + private readonly int DEFAULT_FRAME_FAILURE_THRESHOLD = 250; + public UnloadStrategyHandler(IRealmPartitionSettings realmPartitionSettings, ICacheCleaner cacheCleaner) { this.cacheCleaner = cacheCleaner; - currentUnloadStrategy = 0; - //Higher the index, more aggressive the strategy - unloadStrategies = new IUnloadStrategy[] + //The base strategy at 0 will always run + //On top of that, we adds logic that run only if the previous one fails in an additive manner + unloadStrategies = new UnloadStrategyBase[] { - new StandardUnloadStrategy(), - new ReduceLoadingRadiusUnloadStrategy(realmPartitionSettings), - new UnloadUnusedAssetUnloadStrategy() + new StandardUnloadStrategy(DEFAULT_FRAME_FAILURE_THRESHOLD, cacheCleaner), + new ReduceLoadingRadiusUnloadStrategy(DEFAULT_FRAME_FAILURE_THRESHOLD, realmPartitionSettings), + new UnloadUnusedAssetUnloadStrategy(DEFAULT_FRAME_FAILURE_THRESHOLD) }; } public void TryUnload() { - unloadStrategies[currentUnloadStrategy].TryUnload(cacheCleaner); - if( unloadStrategies[currentUnloadStrategy].FailedOverThreshold()) - IncreaseAggresivenessTier(); + for (var i = unloadStrategies.Length - 1; i >= 0; i--) + { + if (i == 0 || unloadStrategies[i - 1].FaillingOverThreshold()) + unloadStrategies[i].TryUnload(); + } } - public void ResetToNormal() { - currentUnloadStrategy = 0; for (var i = 0; i < unloadStrategies.Length; i++) unloadStrategies[i].ResetStrategy(); } - private void IncreaseAggresivenessTier() - { - currentUnloadStrategy = Math.Clamp(currentUnloadStrategy + 1, 0, unloadStrategies.Length - 1); - } } } \ No newline at end of file diff --git a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadUnusedAssetUnloadStrategy.cs b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadUnusedAssetUnloadStrategy.cs index 865f9e427b..b189d47200 100644 --- a/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadUnusedAssetUnloadStrategy.cs +++ b/Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadUnusedAssetUnloadStrategy.cs @@ -2,19 +2,18 @@ namespace DCL.ResourcesUnloading.UnloadStrategies { - public class UnloadUnusedAssetUnloadStrategy : UnloadStrategy + public class UnloadUnusedAssetUnloadStrategy : UnloadStrategyBase { - - private int FRAMES_UNTIL_UNLOAD_IS_INVOKED = 1000; + private readonly int FRAMES_UNTIL_UNLOAD_IS_INVOKED = 5_000; private int currentFrameCountForUnloadAssets; - - public UnloadUnusedAssetUnloadStrategy() + + public UnloadUnusedAssetUnloadStrategy(int failureThreshold) : base(failureThreshold) { //We equalize it so its invoked on first invocation of TryUnload currentFrameCountForUnloadAssets = FRAMES_UNTIL_UNLOAD_IS_INVOKED; } - public override void TryUnload(ICacheCleaner cacheCleaner) + public override void RunStrategy() { currentFrameCountForUnloadAssets++; if (currentFrameCountForUnloadAssets > FRAMES_UNTIL_UNLOAD_IS_INVOKED) @@ -22,14 +21,14 @@ public override void TryUnload(ICacheCleaner cacheCleaner) Resources.UnloadUnusedAssets(); currentFrameCountForUnloadAssets = 0; } - base.TryUnload(cacheCleaner); } - + public override void ResetStrategy() { base.ResetStrategy(); currentFrameCountForUnloadAssets = FRAMES_UNTIL_UNLOAD_IS_INVOKED; } + } } \ No newline at end of file