Adding Baseframework of the ResourceManager (all hooks and observers) are being put in place with a…

Adding Baseframework of the ResourceManager (all hooks and observers)
are being put in place with a very basic management scheme

The current ash implementation of the LowMemoryObserver polls 
"/dev/chromeos-low-mem" to find if we are running out of memory.
After discussion with semenzato@ we decided that that is not precise
enough and we should rather poll the system memory information. Going
forward this has multiple benefits:
 1. it does not require special kernel handling.
 2. it also gives us information about GPU memory on chromeOS which
    will come handy once we are adding more sophisticated management.

Beside that, this CL adds the ResourceManager which hooks itself into
several components to detect changes in Activity's and their ordering
as well as memory pressure.

Note: The current management implementation itself is really primitive
and needs to be improved.

TBR for the DEPS file

BUG=381212, 403782
TEST=MemoryPressureTest.*, ResourceManagerTest.*
TBR=oshima@chromium.org

Committed: https://crrev.com/c8c38693393c172a44abdd18e4232ed1479be9bd
Cr-Commit-Position: refs/heads/master@{#292286}

[Mr4D (OOO till 08-26), 2014-08-26 23:33:29]

skuhne@chromium.org changed reviewers:
+ mukai@chromium.org

[Mr4D (OOO till 08-26), 2014-08-26 23:41:32]

Please have a look!

[Jun Mukai, 2014-08-27 01:21:19]

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/memory_pressure_notifier.cc (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/memory_pressure_notifier.cc:24: class
MemoryPressureNotifierImpl
You don't have to introduce pimpl pattern to this file.
We separate Impl and public/ only when the file is exported to other athena
modules.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/public/resource_manager.h:21: // ResourceManager.
Seems delegate is not passed?  Please update the comment.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager_delegate.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/public/resource_manager_delegate.h:1: // Copyright 2014
The Chromium Authors. All rights reserved.
This file doesn't have to be public. Please move to athena/resource_manager

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/resource_manager.cc (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
In athena, usually this file is named as resoure_manager_impl.cc because it's
for Impl subclass.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:60: // for us, we can remove this
code here.
Also, it's better to be unified with WindowListProvider in athena/wm...
They do very similar thing in different places.
Better to consult with oshima for that.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:92: void
ResourceManagerImpl::SetMemoryPressureForTest(
It might be great to cancel memory_pressure_notifier_ in testing environment?

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:162: // No need to do anything yet.
Not sure the difference between HIGH/UNKNOWN (setting to 5 explicitly) and LOW.
Do you expect the default value could be changed from 5?
Also, setting value in UNKNOWN doesn't look right. Is it intentional?

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:181:
unloadable_activities.back()->SetCurrentState(Activity::ACTIVITY_UNLOADED);
Is it enough to set the state on only the oldest unloadable activity?
I would expect that:
for (size_t i = max_running_activities; i < unloadable_activities.size(); ++i) {
  unloadable_activities[i]->SetCurrentState(UNLOADED);
}

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:183: } else if
(current_memory_pressure_ == MEMORY_PRESSURE_CRITICAL) {
Think that there are multiple media activities and pressure is CRITICAL.
This logic unloads the oldest one but keeps other media activities. Is that
okay?

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:200: if (!activity_list_.size())
activity_list_.empty() is clearer to me

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:204:
activity_list_[0]->GetWindow()->parent()->children();;
double semicolon at end

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
File athena/wm/public/window_manager_observer.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
athena/wm/public/window_manager_observer.h:23: virtual void
OnActivityOrderHasChanged() = 0;
Should we use wm::ActivationChangeObserver instead? (see
ui/wm/public/activation_change_observer.h)

[Mr4D (OOO till 08-26), 2014-08-27 16:12:14]

Addressed! Please have another look!

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/memory_pressure_notifier.cc (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/memory_pressure_notifier.cc:24: class
MemoryPressureNotifierImpl
Done.  (Did that because of the thread safe release - which isn't required
anymore since my inital IPC code implementation did go away).

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/public/resource_manager.h:21: // ResourceManager.
Indeed! Done.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager_delegate.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/public/resource_manager_delegate.h:1: // Copyright 2014
The Chromium Authors. All rights reserved.
Actually... It is used by the testing framework & some tests to override.
Looking at athena/test/DEPS this file has to come out of public (unless I have
missed something?).

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/resource_manager.cc (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
On 2014/08/27 01:21:19, Jun Mukai wrote:
> In athena, usually this file is named as resoure_manager_impl.cc because it's
> for Impl subclass.

Done.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:60: // for us, we can remove this
code here.
I know. I have a CL queued (480293003) which adds more functionality to the
WindowListProvider. I have sent it to Oshima, but he was not yet looking at it.
To get this thing forward I have added it this way and will change once the
other thing lands. I therefore added the TODO.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:92: void
ResourceManagerImpl::SetMemoryPressureForTest(
The TestResourceManagerDelegate will never report any memory utilization and as
such it will never fire. Note furthermore that the timeout is quite high, so
that it would possibly never fire either.

I added some comments to make this clearer.

Nevertheless - I added the call to explicitly stop the timer as well (even
though a third insurance against firing seems over the top :) ).

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:162: // No need to do anything yet.
I added another comment for unknown. You can find more comments about these
values in the header file which explains them, but since this is the part which
needs to be filled with more life, a bit more explanation:

1. This code will need to be changed with a smarter code (see TODO above). Smart
equals: looking at all kinds of input like how much memory is used by each
process, do incremental resource release instead of unload, look into usage
patterns, ...).
2. UNKNOWN means that the kernel does not tell us how much memory he has used /
available. As such we have to assume something bad. As such I have put in there
a HIGH. (let's discuss offline if you have questions about this point).
3. Could this value be changed? Obviously yes! This code is only a temporary
placeholder and will be replaced with more smarts. This is only the first stab
to see some management... :)

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:181:
unloadable_activities.back()->SetCurrentState(Activity::ACTIVITY_UNLOADED);
The crux here is that any resource change takes time to trickle through the
system and it is unclear of how much memory is released. As such, the logic in
ash does unload one tab, wait for ~1s and then checks again if it helped.

A more sophisticated logic might then use the ProcessInfo utilities to estimate
the release, but that will be part of another CL. Also note that this CL does
not yet use the sleeping modes which will also add additional saving methods.

On the other hand - looking at applications, a single app might have multiple
activities and the app only gets unloaded when all activities are marked for
unloading. So in the worst case, several iterations would not turn up any
release. But using the sleep states and other logic will ballance that out (in
most cases).

Again - this is a "dumb" implementation to get started.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:183: } else if
(current_memory_pressure_ == MEMORY_PRESSURE_CRITICAL) {
That was the intent. If there is no memory left and we have to do something we
will give the oldest one the boots. We could however wait until the kernel
forecfully kills one... 

But - we have to release memory and since all are "Active" we need to kill one
which is not visible and plays media.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:200: if (!activity_list_.size())
On 2014/08/27 01:21:18, Jun Mukai wrote:
> activity_list_.empty() is clearer to me

Done.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:204:
activity_list_[0]->GetWindow()->parent()->children();;
On 2014/08/27 01:21:18, Jun Mukai wrote:
> double semicolon at end

Done.

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
File athena/wm/public/window_manager_observer.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
athena/wm/public/window_manager_observer.h:23: virtual void
OnActivityOrderHasChanged() = 0;
Any re-ordering is of interest since "older" activities need adjusting and the
order does define that. If we would have used the container window for
organizing our activities, that could have been done through its WindowObserver
- and / or in that case we might have used the suggested
activation_change_observer and hold the activity order ourselves. However -
Oshima asked to decouple the activity order management from the window
management. Does that make sense?

[Jun Mukai, 2014-08-27 16:57:48]

lgtm with nits

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
File athena/resource_manager/resource_manager.cc (right):

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:60: // for us, we can remove this
code here.
On 2014/08/27 16:12:13, Mr4D wrote:
> I know. I have a CL queued (480293003) which adds more functionality to the
> WindowListProvider. I have sent it to Oshima, but he was not yet looking at
it.
> To get this thing forward I have added it this way and will change once the
> other thing lands. I therefore added the TODO.

Turns out I was still confused by the difference between window and activity.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:92: void
ResourceManagerImpl::SetMemoryPressureForTest(
On 2014/08/27 16:12:13, Mr4D wrote:
> The TestResourceManagerDelegate will never report any memory utilization and
as
> such it will never fire. Note furthermore that the timeout is quite high, so
> that it would possibly never fire either.
> 
> I added some comments to make this clearer.
> 
> Nevertheless - I added the call to explicitly stop the timer as well (even
> though a third insurance against firing seems over the top :) ).

Acknowledged.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:181:
unloadable_activities.back()->SetCurrentState(Activity::ACTIVITY_UNLOADED);
On 2014/08/27 16:12:13, Mr4D wrote:
> The crux here is that any resource change takes time to trickle through the
> system and it is unclear of how much memory is released. As such, the logic in
> ash does unload one tab, wait for ~1s and then checks again if it helped.
> 
> A more sophisticated logic might then use the ProcessInfo utilities to
estimate
> the release, but that will be part of another CL. Also note that this CL does
> not yet use the sleeping modes which will also add additional saving methods.
> 
> On the other hand - looking at applications, a single app might have multiple
> activities and the app only gets unloaded when all activities are marked for
> unloading. So in the worst case, several iterations would not turn up any
> release. But using the sleep states and other logic will ballance that out (in
> most cases).
> 
> Again - this is a "dumb" implementation to get started.

Ah, I see.

https://codereview.chromium.org/513523002/diff/20001/athena/resource_manager/...
athena/resource_manager/resource_manager.cc:183: } else if
(current_memory_pressure_ == MEMORY_PRESSURE_CRITICAL) {
On 2014/08/27 16:12:13, Mr4D wrote:
> That was the intent. If there is no memory left and we have to do something we
> will give the oldest one the boots. We could however wait until the kernel
> forecfully kills one... 
> 
> But - we have to release memory and since all are "Active" we need to kill one
> which is not visible and plays media.

Acknowledged.

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
File athena/wm/public/window_manager_observer.h (right):

https://codereview.chromium.org/513523002/diff/20001/athena/wm/public/window_...
athena/wm/public/window_manager_observer.h:23: virtual void
OnActivityOrderHasChanged() = 0;
On 2014/08/27 16:12:14, Mr4D wrote:
> Any re-ordering is of interest since "older" activities need adjusting and the
> order does define that. If we would have used the container window for
> organizing our activities, that could have been done through its
WindowObserver
> - and / or in that case we might have used the suggested
> activation_change_observer and hold the activity order ourselves. However -
> Oshima asked to decouple the activity order management from the window
> management. Does that make sense?

I see. Makes sense.

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp#newcod...
athena/athena.gyp:156: 'resource_manager/public/resource_manager_delegate.h',
public/resource_manager_delegate.h already appears in athena_content_lib

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp#newcod...
athena/athena.gyp:181: 'resource_manager/public/resource_manager_delegate.h',
ditto -- this file already appears in athena_content_lib

https://codereview.chromium.org/513523002/diff/40001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager_delegate.h (right):

https://codereview.chromium.org/513523002/diff/40001/athena/resource_manager/...
athena/resource_manager/public/resource_manager_delegate.h:28: virtual int
MemoryPressureIntervalInMS() = 0;
possibly better to return base::TimeDelta instead?

[Mr4D (OOO till 08-26), 2014-08-27 18:09:38]

Many thanks for your quick review!

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp#newcod...
athena/athena.gyp:156: 'resource_manager/public/resource_manager_delegate.h',
On 2014/08/27 16:57:48, Jun Mukai wrote:
> public/resource_manager_delegate.h already appears in athena_content_lib

Done.

https://codereview.chromium.org/513523002/diff/40001/athena/athena.gyp#newcod...
athena/athena.gyp:181: 'resource_manager/public/resource_manager_delegate.h',
On 2014/08/27 16:57:48, Jun Mukai wrote:
> ditto -- this file already appears in athena_content_lib

Done.

https://codereview.chromium.org/513523002/diff/40001/athena/resource_manager/...
File athena/resource_manager/public/resource_manager_delegate.h (right):

https://codereview.chromium.org/513523002/diff/40001/athena/resource_manager/...
athena/resource_manager/public/resource_manager_delegate.h:28: virtual int
MemoryPressureIntervalInMS() = 0;
Having "real ms's" value seems to be much better to look at / debug than having
a TimeDelta. As such I prefer to rather leave it this way. Ack.

[Mr4D (OOO till 08-26), 2014-08-27 18:09:43]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 18:11:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/60001

[commit-bot: I haz the power, 2014-08-27 18:26:23]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47095)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/52454)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-27 18:28:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-27 18:28:05]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47102)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mr4D (OOO till 08-26), 2014-08-27 18:58:28]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 18:59:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/80001

[commit-bot: I haz the power, 2014-08-27 20:29:54]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-27 20:33:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-27 20:33:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Mr4D (OOO till 08-26), 2014-08-27 20:48:05]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 20:49:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/100001

[Mr4D (OOO till 08-26), 2014-08-27 21:11:15]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 21:12:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/120001

[Luigi Semenzato, 2014-08-27 21:23:55]

On 2014/08/27 21:12:51, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/120001

I just found out about this and I am a little confused.  I was hoping to have
communicated that polling /proc/meminfo would be a good way to test the
concepts, but I am not sure it's a good permanent implementation.  Here are some
of the reasons.

1. Polling at too fast a rate will make it harder for the system to go into a
quiescent state, so it will use more power.

2. Polling at too slow a rate will reduce the speed at which we respond to
memory state changes.

3. Too slow and too fast may intersect.

4. The correct memory pressure levels may depend on the system.  In the current
framework, we have at least three different settings (4 GB, 2 GB, 2 GB ARM) and
the number of settings may grow.  It is probably preferable to abstract this
away from chrome.

I was also suggesting to use /proc/meminfo to compute the memory pressure in a
way similar to how it is done now.  The way this patch computes memory pressure
is incorrect.

[Luigi Semenzato, 2014-08-27 21:24:08]

semenzato@chromium.org changed reviewers:
+ semenzato@chromium.org

[Luigi Semenzato, 2014-08-27 21:24:08]

https://codereview.chromium.org/513523002/diff/100001/athena/resource_manager...
File athena/resource_manager/delegate/resource_manager_delegate.cc (right):

https://codereview.chromium.org/513523002/diff/100001/athena/resource_manager...
athena/resource_manager/delegate/resource_manager_delegate.cc:31: return
((memory.total - memory.free) * 100) / memory.total;
Is memory.free the same as the MemFree field of /proc/meminfo?  I thought we
discussed this.  That number is not particularly useful for your purposes.

[Mr4D (OOO till 08-26), 2014-08-27 22:14:51]

The CQ bit was unchecked by skuhne@chromium.org

[Mr4D (OOO till 08-26), 2014-08-27 22:34:01]

Okay. Will address the memory calculation in a separate patch. For the long term
usage - we can change that, but for now we need something to get started. Will
send you a separate patch for the memory calculation change.

https://codereview.chromium.org/513523002/diff/100001/athena/resource_manager...
File athena/resource_manager/delegate/resource_manager_delegate.cc (right):

https://codereview.chromium.org/513523002/diff/100001/athena/resource_manager...
athena/resource_manager/delegate/resource_manager_delegate.cc:31: return
((memory.total - memory.free) * 100) / memory.total;
Okay, I will change that in a separate patch. Let's discuss what to use (somehow
I was under the impression that I should use /proc/meminfo).

[Mr4D (OOO till 08-26), 2014-08-27 22:34:10]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 22:35:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/140001

[commit-bot: I haz the power, 2014-08-27 23:36:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[commit-bot: I haz the power, 2014-08-27 23:39:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-27 23:39:37]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Mr4D (OOO till 08-26), 2014-08-27 23:47:51]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-08-27 23:48:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/513523002/160001

[commit-bot: I haz the power, 2014-08-28 00:47:48]

Committed patchset #9 (id:160001) as 06da2746706f7168ccb69923bac1237957b12297

[oshima, 2014-09-03 22:12:29]

oshima@chromium.org changed reviewers:
+ oshima@chromium.org

[oshima, 2014-09-03 22:12:30]

https://codereview.chromium.org/513523002/diff/160001/athena/wm/public/window...
File athena/wm/public/window_manager_observer.h (right):

https://codereview.chromium.org/513523002/diff/160001/athena/wm/public/window...
athena/wm/public/window_manager_observer.h:23: virtual void
OnActivityOrderHasChanged() = 0;
FYI: WM should not have notion of activity.
And this should probably be an observer to the WLP (or whatever the appropriate
name is), not WM itself.

[commit-bot: I haz the power, 2014-09-10 02:56:15]

Patchset 9 (id:??) landed as
https://crrev.com/c8c38693393c172a44abdd18e4232ed1479be9bd
Cr-Commit-Position: refs/heads/master@{#292286}