A visibile to invisible state change will also release memory and should therefore wait as an unloa…

athena/resource_manager/resource_manager_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "athena/resource_manager/public/resource_manager.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "athena/activity/public/activity.h"
@@ skipping 63 matching lines @@
 
   // WindowListProviderObserver:
   virtual void OnWindowStackingChanged() OVERRIDE;
   virtual void OnWindowRemoved(aura::Window* removed_window,
                                int index) OVERRIDE;
 
  private:
   // Manage the resources for our activities.
   void ManageResource();
 
+  // Check that the visibility of activities is properly set.
+  void CheckVisibilityStates();
+
+  // Check if activities can be unloaded to reduce memory pressure.
+  void TryToUnloadAnActivity();
+
   // Order our activity list to the order of activities of the stream.
   // TODO(skuhne): Once the ActivityManager is responsible to create this list
   // for us, we can remove this code here.
   void UpdateActivityOrder();
 
+  // Resources were released and a quiet period is needed before we release
+  // more since it takes a while to trickle through the system.
+  void ResourcesGotReleased();

[oshima, 2014/09/22 16:22:51]

OnResourceReleased is more common.

[Mr4D (OOO till 08-26), 2014/09/23 00:28:56]

Done.

[Even though I have to say that the On.. notion feels more like an external
event and not like an internal function]

+
+  // The memory pressure has increased, previously applied measures did not show
+  // effect and immediate action is required.
+  void MemoryPressureIncreased();

[oshima, 2014/09/22 16:22:51]

ditto. OnMemoryPressureIncreased

[Mr4D (OOO till 08-26), 2014/09/23 00:28:56]

Done. And ditto.

+
+  // Returns true when the previous memory release was long enough ago to try
+  // unloading another activity.
+  bool AllowedToUnloadActivity();
+
   // The sorted (new(front) -> old(back)) activity list.
   // TODO(skuhne): Once the ActivityManager is responsible to create this list
   // for us, we can remove this code here.
   std::vector<Activity*> activity_list_;
 
   // The resource manager delegate.
   scoped_ptr<ResourceManagerDelegate> delegate_;
 
   // Keeping a reference to the current memory pressure.
   MemoryPressureObserver::MemoryPressure current_memory_pressure_;
@@ skipping 135 matching lines @@
   // Manage the resources of each activity.
   ManageResource();
 }
 
 void ResourceManagerImpl::OnWindowRemoved(aura::Window* removed_window,
                                           int index) {
 }
 
 void ResourceManagerImpl::OnMemoryPressure(
       MemoryPressureObserver::MemoryPressure pressure) {
+  if (pressure > current_memory_pressure_)
+    MemoryPressureIncreased();
   current_memory_pressure_ = pressure;
   ManageResource();
 }
 
 ResourceManagerDelegate* ResourceManagerImpl::GetDelegate() {
   return delegate_.get();
 }
 
 void ResourceManagerImpl::ManageResource() {
   // If there is none or only one app running we cannot do anything.
   if (activity_list_.size() <= 1U)
     return;
 
   if (pause_) {
     queued_command_ = true;
     return;
   }
 
-  // TODO(skuhne): This algorithm needs to take all kinds of predictive analysis
-  // and running applications into account. For this first patch we only do a
-  // very simple "floating window" algorithm which is surely not good enough.
-  size_t max_running_activities = 5;
-  switch (current_memory_pressure_) {
-    case MEMORY_PRESSURE_UNKNOWN:
-      // If we do not know how much memory we have we assume that it must be a
-      // high consumption.
-      // Fallthrough.
-    case MEMORY_PRESSURE_HIGH:
-      max_running_activities = 5;
-      break;
-    case MEMORY_PRESSURE_CRITICAL:
-      max_running_activities = 0;
-      break;
-    case MEMORY_PRESSURE_MODERATE:
-      max_running_activities = 7;
-      break;
-    case MEMORY_PRESSURE_LOW:
-      // This doesn't really matter. We do not change anything but turning
-      // activities visible.
-      max_running_activities = 10000;
-      break;
-  }
+  // Check that the visibility of items is properly set. Note that this might
+  // already trigger a release of resources. If this happens,
+  // AllowedToUnloadActivity() will return false.
+  CheckVisibilityStates();
 
+  // Since resource deallocation takes time, we avoid to release more resources
+  // in short succession. Note that we come here periodically and if one call
+  // is not triggering an unload, the next one will.
+  if (AllowedToUnloadActivity())
+    TryToUnloadAnActivity();
+}
+
+void ResourceManagerImpl::CheckVisibilityStates() {

[oshima, 2014/09/22 16:22:50]

how about UpdateVisibilityStates?

[Mr4D (OOO till 08-26), 2014/09/23 00:28:56]

Done.

   // The first n activities should be treated as "visible", means they updated
   // in overview mode and will keep their layer resources for faster switch
   // times. Usually we use |kMaxVisibleActivities| items, but when the memory
   // pressure gets critical we only hold as many as are really visible.
   size_t max_activities = kMaxVisibleActivities;
   if (current_memory_pressure_ == MEMORY_PRESSURE_CRITICAL)
     max_activities = 1 + (in_splitview_mode_ ? 1 : 0);

[oshima, 2014/09/22 16:22:51]

in_spit_view_mode ? 2 : 1

[Mr4D (OOO till 08-26), 2014/09/23 00:28:56]

Done.

 
   // Restart and / or bail if the order of activities changes due to our calls.
   activity_order_changed_ = false;
 
   // Change the visibility of our activities in a pre-processing step. This is
   // required since it might change the order/number of activities.
   size_t index = 0;
   while (index < activity_list_.size()) {
     Activity* activity = activity_list_[index];
     Activity::ActivityState state = activity->GetCurrentState();
 
     // The first |kMaxVisibleActivities| entries should be visible, all others
     // invisible or at a lower activity state.
     if (index < max_activities ||
         (state == Activity::ACTIVITY_INVISIBLE ||
          state == Activity::ACTIVITY_VISIBLE)) {
       Activity::ActivityState visiblity_state =
           index < max_activities ? Activity::ACTIVITY_VISIBLE :
                                    Activity::ACTIVITY_INVISIBLE;
       // Only change the state when it changes. Note that when the memory
       // pressure is critical, only the primary activities (1 or 2) are made
       // visible. Furthermore, in relaxed mode we only want to turn visible,
       // never invisible.
       if (visiblity_state != state &&
           (current_memory_pressure_ != MEMORY_PRESSURE_LOW ||
            visiblity_state == Activity::ACTIVITY_VISIBLE)) {
         activity->SetCurrentState(visiblity_state);
-        // If we turned an activity invisible, we should not at the same time
-        // throw an activity out of memory. Thus we grant one more invisible
-        // Activity in that case.
+        // If we turned an activity invisible, we are already releasing memory
+        // and can hold off releasing more for now.
         if (visiblity_state == Activity::ACTIVITY_INVISIBLE)
-          max_running_activities++;
+          ResourcesGotReleased();
       }
     }
 
     // See which index we should handle next.
     if (activity_order_changed_) {
       activity_order_changed_ = false;
       index = 0;
     } else {
       ++index;
     }
   }
+}
 
-  // If there is only a low memory pressure, or our last call to release
-  // resources cannot have had any impact yet, we return.
-  // TODO(skuhne): The upper part of this function bumps up the state (to
-  // visible) and the lower part might unload one. Going forward this algorithm
-  // will change significantly and when it does we might want to break this into
-  // two separate pieces.
-  if (current_memory_pressure_ == MEMORY_PRESSURE_LOW ||
-      base::Time::Now() < next_resource_management_time_)
-    return;
-  // Do not release too many activities in short succession since it takes time
-  // to release resources. As such wait the memory pressure interval before the
-  // next call.
-  next_resource_management_time_ = base::Time::Now() +
-                                   wait_time_for_resource_deallocation_;
+void ResourceManagerImpl::TryToUnloadAnActivity() {
+  // TODO(skuhne): This algorithm needs to take all kinds of predictive analysis
+  // and running applications into account. For this first patch we only do a
+  // very simple "floating window" algorithm which is surely not good enough.
+  size_t max_running_activities = 5;
+  switch (current_memory_pressure_) {
+    case MEMORY_PRESSURE_UNKNOWN:
+      // If we do not know how much memory we have we assume that it must be a
+      // high consumption.
+      // Fallthrough.
+    case MEMORY_PRESSURE_HIGH:
+      max_running_activities = 5;
+      break;
+    case MEMORY_PRESSURE_CRITICAL:
+      max_running_activities = 0;
+      break;
+    case MEMORY_PRESSURE_MODERATE:
+      max_running_activities = 7;
+      break;
+    case MEMORY_PRESSURE_LOW:
+      NOTREACHED();
+      return;
+  }
 
-  // Check if/which activity we want to unload.
+  // Check if / which activity we want to unload.
   Activity* oldest_media_activity = NULL;
   std::vector<Activity*> unloadable_activities;
   for (std::vector<Activity*>::iterator it = activity_list_.begin();
        it != activity_list_.end(); ++it) {
     Activity::ActivityState state = (*it)->GetCurrentState();
     // The activity should neither be unloaded nor visible.
     if (state != Activity::ACTIVITY_UNLOADED &&
         state != Activity::ACTIVITY_VISIBLE) {
       if ((*it)->GetMediaState() == Activity::ACTIVITY_MEDIA_STATE_NONE) {
         // Does not play media - so we can unload this immediately.
         unloadable_activities.push_back(*it);
       } else {
         oldest_media_activity = *it;
       }
     }
   }
 
   if (unloadable_activities.size() > max_running_activities) {
+    ResourcesGotReleased();
     unloadable_activities.back()->SetCurrentState(Activity::ACTIVITY_UNLOADED);
     return;
   } else if (current_memory_pressure_ == MEMORY_PRESSURE_CRITICAL) {
     if (oldest_media_activity) {
+      ResourcesGotReleased();
       oldest_media_activity->SetCurrentState(Activity::ACTIVITY_UNLOADED);
       LOG(WARNING) << "Unloading item to releave critical memory pressure";
       return;
     }
     LOG(ERROR) << "[ResourceManager]: Single activity uses too much memory.";
     return;
   }
 
   if (current_memory_pressure_ != MEMORY_PRESSURE_UNKNOWN) {
     // Only show this warning when the memory pressure is actually known. This
@@ skipping 29 matching lines @@
     }
   }
   // At this point the old list should be empty and we can swap the lists.
   DCHECK(!activity_list_.size());
   activity_list_ = new_activity_list;
 
   // Remember that the activity order has changed.
   activity_order_changed_ = true;
 }
 
+void ResourceManagerImpl::ResourcesGotReleased() {
+  // Do not release too many activities in short succession since it takes time
+  // to release resources. As such wait the memory pressure interval before the
+  // next call.
+  next_resource_management_time_ = base::Time::Now() +
+                                   wait_time_for_resource_deallocation_;
+}
+
+void ResourceManagerImpl::MemoryPressureIncreased() {
+  // By setting the timer to Now, the next call will immediately be performed.
+  next_resource_management_time_ = base::Time::Now();

[oshima, 2014/09/22 16:22:50]

Note that Now() may not always increase (that is, may go back), so the next
AllowedToUnloadActivty still may return false.

It's rare so this shouldn't be an issue, but just wanted to point out.

[Mr4D (OOO till 08-26), 2014/09/23 00:28:56]

Which would be ok! All which is required is that ResourcesGotReleased() is
setting it to a later time. So no problem!

[oshima, 2014/09/23 00:42:35]

Just in case you might have misunderstood my comment. Now() in
AllowedToUnloadActivity may return earlier time than Now() here, so the next
call may not execute immediately. (It's extremely rare so this won't be an
issue, as I said)

+}
+
+bool ResourceManagerImpl::AllowedToUnloadActivity() {
+  return current_memory_pressure_ != MEMORY_PRESSURE_LOW &&
+         base::Time::Now() >= next_resource_management_time_;
+}
+
 // static
 void ResourceManager::Create() {
   DCHECK(!instance);
   instance = new ResourceManagerImpl(
       ResourceManagerDelegate::CreateResourceManagerDelegate());
 }
 
 // static
 ResourceManager* ResourceManager::Get() {
   return instance;
 }
 
 // static
 void ResourceManager::Shutdown() {
   DCHECK(instance);
   delete instance;
   instance = NULL;
 }
 
 ResourceManager::ResourceManager() {}
 
 ResourceManager::~ResourceManager() {
   DCHECK(instance);
   instance = NULL;
 }
 
 }  // namespace athena