Make task manager memory data more efficient and meaningful.

chrome/browser/task_manager/task_manager.cc

Index: chrome/browser/task_manager/task_manager.cc
diff --git a/chrome/browser/task_manager/task_manager.cc b/chrome/browser/task_manager/task_manager.cc
index 11d58fce00ee2c88cbf5e238ba9d27c1f61ff02e..7c68eb7c4597e206f5d060fd35c50409535102a7 100644
--- a/chrome/browser/task_manager/task_manager.cc
+++ b/chrome/browser/task_manager/task_manager.cc
@@ -10,7 +10,7 @@
 #include "base/location.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
-#include "base/process/process_metrics.h"
+#include "base/process/private_working_set_snapshot.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
@@ -274,6 +274,11 @@ TaskManagerModel::TaskManagerModel(TaskManager* task_manager)
       task_manager,
       scoped_ptr<WebContentsInformation>(
           new task_manager::GuestInformation())));
+#if defined(OS_WIN)
+  working_set_snapshot_.reset(new base::PrivateWorkingSetSnapshot);
+  working_set_snapshot_->AddToMonitorList("chrome");
+  working_set_snapshot_->AddToMonitorList("nacl64");
+#endif
 }
 
 void TaskManagerModel::AddObserver(TaskManagerModelObserver* observer) {
@@ -578,10 +583,13 @@ bool TaskManagerModel::GetPhysicalMemory(int index, size_t* result) const {
     // On Linux private memory is also resident. Just use it.
     values.physical_memory = ws_usage.priv * 1024;
 #else
-    // Memory = working_set.private + working_set.shareable.
-    // We exclude the shared memory.
+    // Memory = working_set.private which is working set minus shareable. This
+    // avoids the unpredictable counting that occurs when calculating memory as
+    // working set minus shared (renderer code counted when one tab is open and
+    // not counted when two or more are open) and it is much more efficient to
+    // calculate on Windows.
     values.physical_memory = iter->second->GetWorkingSetSize();
-    values.physical_memory -= ws_usage.shared * 1024;
+    values.physical_memory -= ws_usage.shareable * 1024;
 #endif
   }
   *result = values.physical_memory;
@@ -1123,9 +1131,35 @@ void TaskManagerModel::ModelChanged() {
   FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_, OnModelChanged());
 }
 
+void TaskManagerModel::PopulateCache() {

[ncarter (slow), 2015/06/26 21:46:16]

Add a comment here (or at the bottom of the function) to describe why doing
nothing is okay on non-windows.  You could put it, if you want, in a comment in
an (otherwise empty) #else clause.

[brucedawson, 2015/06/27 00:13:06]

Done.

+#if defined(OS_WIN)
+  // Collect working-set data for all monitored processes in one operation, to
+  // avoid the inefficiency of retrieving it one at a time.
+  working_set_snapshot_->Sample();
+
+  for (size_t rIndex = 0; rIndex < resources_.size(); ++rIndex) {

[ncarter (slow), 2015/06/26 21:46:16]

 "i", "index", "resource", or "resource_index", but definitely not "rIndex"

[brucedawson, 2015/06/27 00:13:06]

Done.

+    size_t privateWS = working_set_snapshot_->GetPrivateWorkingSet(

[ncarter (slow), 2015/06/26 21:46:17]

private_working_set

[brucedawson, 2015/06/27 00:13:06]

Done.

+        GetProcessId(rIndex));
+
+    // If working-set data is available then use it. If not then
+    // GetWorkingSetKBytes will retrieve the data. This is rare except on
+    // Windows XP where GetWorkingSetKBytes will always be used.
+    if (privateWS) {
+      // Fill in the cache with the retrieved private working set value.
+      base::ProcessHandle handle = GetResource(rIndex)->GetProcess();
+      PerProcessValues& values(per_process_cache_[handle]);
+      values.is_physical_memory_valid = true;
+      // Note that the other memory fields are *not* filled in.
+      values.physical_memory = privateWS;
+    }
+  }
+#endif
+}
+
 void TaskManagerModel::Refresh() {
   per_resource_cache_.clear();
   per_process_cache_.clear();
+  PopulateCache();

[ncarter (slow), 2015/06/26 21:46:16]

Maybe: RefreshPhysicalMemoryFromWorkingSetSnapshot()

[brucedawson, 2015/06/27 00:13:06]

Done.

 
 #if !defined(DISABLE_NACL)
   nacl::NaClBrowser* nacl_browser = nacl::NaClBrowser::GetInstance();