chore: UnloadStrategy abstract class (#2508)

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;

Explorer/Assets/DCL/ResourcesUnloading/Tests/ReleaseMemorySystemShould.cs

@@ -10,17 +10,31 @@ namespace DCL.ResourcesUnloading.Tests
 {
     public class ReleaseMemorySystemShould : UnitySystemTestBase<ReleaseMemorySystem>
     {
+        public class MockUnloadStrategy : UnloadStrategyBase
+        {
+            public int strategyRunCount;
+
+            public override void RunStrategy()
+            {
+                strategyRunCount++;
+            }
+
+            public MockUnloadStrategy(int failureThreshold) : base(failureThreshold)
+            {
+            }
+        }
+
 
         private ReleaseMemorySystem releaseMemorySystem;
 
         // Subs
         private IMemoryUsageProvider memoryBudgetProvider;
         private ICacheCleaner cacheCleaner;
 
-        private IUnloadStrategy[] unloadStrategies;
+        private UnloadStrategyBase[] unloadStrategies;
 
-        private IUnloadStrategy standardStrategy;
-        private IUnloadStrategy aggresiveStrategy;
+        private MockUnloadStrategy standardStrategy;
+        private MockUnloadStrategy aggresiveStrategy;
 
         private UnloadStrategyHandler unloadStrategyHandler;
 
@@ -29,8 +43,8 @@ public void SetUp()
         {
             memoryBudgetProvider = Substitute.For<IMemoryUsageProvider>();
             cacheCleaner = Substitute.For<ICacheCleaner>();
-            standardStrategy = Substitute.For<IUnloadStrategy>();
-            aggresiveStrategy = Substitute.For<IUnloadStrategy>();
+            standardStrategy = new MockUnloadStrategy(1);
+            aggresiveStrategy = new MockUnloadStrategy(1);
 
             unloadStrategies = new[]
             {
@@ -52,79 +66,79 @@ 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]
         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
             memoryBudgetProvider.GetMemoryUsageStatus().Returns(MemoryUsageStatus.NORMAL);
             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]
         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);
         }
 
 

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);
         }
 
     }

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;
+        }
     }
 }

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++;
+        }
+    }
+}

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);
-        }
     }
 }

Explorer/Assets/DCL/ResourcesUnloading/UnloadStrategies/UnloadUnusedAssetUnloadStrategy.cs

@@ -2,34 +2,33 @@
 
 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)
             {
                 Resources.UnloadUnusedAssets();
                 currentFrameCountForUnloadAssets = 0;
             }
-            base.TryUnload(cacheCleaner);
         }
-        
+
         public override void ResetStrategy()
         {
             base.ResetStrategy();
             currentFrameCountForUnloadAssets = FRAMES_UNTIL_UNLOAD_IS_INVOKED;
         }
 
+
     }
 }