Add function to compute proportional set size for OS_WIN

base/process/process_metrics_win.cc

 // Copyright (c) 2013 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 "base/process/process_metrics.h"
 
 #include <windows.h>
 #include <psapi.h>
 #include <stddef.h>
 #include <stdint.h>
@@ skipping 127 matching lines @@
   }
   usage->image = committed_image / 1024;
   usage->mapped = committed_mapped / 1024;
   usage->priv = committed_private / 1024;
 }
 
 namespace {
 
 class WorkingSetInformationBuffer {
  public:
-  WorkingSetInformationBuffer() {}
+  WorkingSetInformationBuffer() { number_of_entries = 0; }
   ~WorkingSetInformationBuffer() { Clear(); }
 
   bool Reserve(size_t size) {
     Clear();
     // Use UncheckedMalloc here because this can be called from the code
     // that handles low memory condition.
     return UncheckedMalloc(size, reinterpret_cast<void**>(&buffer_));
   }
 
-  PSAPI_WORKING_SET_INFORMATION* get() { return buffer_; }
   const PSAPI_WORKING_SET_INFORMATION* operator ->() const { return buffer_; }
 
+  DWORD GetPageEntriesNumber() { return number_of_entries; }

[brucedawson, 2016/12/06 02:58:55]

Should be const.

[chengx, 2016/12/06 19:40:09]

Done.

+
+  // This function is used to get page entries for a process.
+  bool GetPageEntries(const ProcessHandle& process_) {
+    int retries = 5;
+    number_of_entries = 4096;  // Just a guess.
+
+    for (;;) {
+      DWORD buffer_size = sizeof(PSAPI_WORKING_SET_INFORMATION) +
+                          (number_of_entries * sizeof(PSAPI_WORKING_SET_BLOCK));
+
+      if (!Reserve(buffer_size))
+        return false;
+
+      // Call the function once to get number of items
+      if (QueryWorkingSet(process_, buffer_, buffer_size))
+        break;  // Success
+
+      if (GetLastError() != ERROR_BAD_LENGTH)
+        return false;
+
+      number_of_entries = static_cast<DWORD>(buffer_->NumberOfEntries);
+
+      // Maybe some entries are being added right now. Increase the buffer to
+      // take that into account. Increasing by 10% should generally be enough,
+      // especially considering the potentially low memory condition during the
+      // call (when called from OomMemoryDetails) and the potentially high
+      // number of entries (300K was observed in crash dumps).
+      number_of_entries = static_cast<DWORD>(number_of_entries * 1.1);
+
+      if (--retries == 0) {
+        // If we're looping, eventually fail.
+        return false;
+      }
+    }
+
+    // On windows 2000 the function returns 1 even when the buffer is too small.
+    // The number of entries that we are going to parse is the minimum between
+    // the

[Primiano Tucci (use gerrit), 2016/12/06 17:47:57]

I think clang-format did bite you here (It's a KI it's not that good in
re-wrapping pre-wrapped comments)

[chengx, 2016/12/06 19:40:09]

Acknowledged.

+    // size we allocated and the real number of entries.
+    number_of_entries = std::min(number_of_entries,
+                                 static_cast<DWORD>(buffer_->NumberOfEntries));
+
+    return true;
+  }
+
  private:
   void Clear() {
     free(buffer_);
     buffer_ = nullptr;
   }
 
   PSAPI_WORKING_SET_INFORMATION* buffer_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(WorkingSetInformationBuffer);
+
+  // Number of page entries.
+  DWORD number_of_entries;

[brucedawson, 2016/12/06 02:58:55]

Could/should go "= 0;" here rather than listing the initialization in the
constructor. New C++11 feature, and consistent with how buffer_ is initialized.

[chengx, 2016/12/06 19:40:09]

Done.

 };
 
 }  // namespace
 
 bool ProcessMetrics::GetWorkingSetKBytes(WorkingSetKBytes* ws_usage) const {
   size_t ws_private = 0;
   size_t ws_shareable = 0;
   size_t ws_shared = 0;
 
   DCHECK(ws_usage);
   memset(ws_usage, 0, sizeof(*ws_usage));
 
-  DWORD number_of_entries = 4096;  // Just a guess.
   WorkingSetInformationBuffer buffer;
-  int retries = 5;
-  for (;;) {
-    DWORD buffer_size = sizeof(PSAPI_WORKING_SET_INFORMATION) +
-                        (number_of_entries * sizeof(PSAPI_WORKING_SET_BLOCK));
+  if (!buffer.GetPageEntries(process_))
+    return false;
 
-    if (!buffer.Reserve(buffer_size))
-      return false;
-
-    // Call the function once to get number of items
-    if (QueryWorkingSet(process_, buffer.get(), buffer_size))
-      break;  // Success
-
-    if (GetLastError() != ERROR_BAD_LENGTH)
-      return false;
-
-    number_of_entries = static_cast<DWORD>(buffer->NumberOfEntries);
-
-    // Maybe some entries are being added right now. Increase the buffer to
-    // take that into account. Increasing by 10% should generally be enough,
-    // especially considering the potentially low memory condition during the
-    // call (when called from OomMemoryDetails) and the potentially high
-    // number of entries (300K was observed in crash dumps).
-    number_of_entries = static_cast<DWORD>(number_of_entries * 1.1);
-
-    if (--retries == 0) {
-      // If we're looping, eventually fail.
-      return false;
-    }
-  }
-
-  // On windows 2000 the function returns 1 even when the buffer is too small.
-  // The number of entries that we are going to parse is the minimum between the
-  // size we allocated and the real number of entries.
-  number_of_entries =
-      std::min(number_of_entries, static_cast<DWORD>(buffer->NumberOfEntries));
-  for (unsigned int i = 0; i < number_of_entries; i++) {
+  for (unsigned int i = 0; i < buffer.GetPageEntriesNumber(); i++) {

[brucedawson, 2016/12/06 02:58:55]

Should generally avoid calling functions like this in the loop. Not really
important in this case, but it can make life harder for the optimizer. Ditto for
loop below.

Could also be very trendy and use begin()/end() functions and range-based for
loops - ask me for details.

[chengx, 2016/12/06 19:40:09]

After a second thought, I think I will stick with index-based looping for now. 

     if (buffer->WorkingSetInfo[i].Shared) {
       ws_shareable++;
       if (buffer->WorkingSetInfo[i].ShareCount > 1)
         ws_shared++;
     } else {
       ws_private++;
     }
   }
 
   ws_usage->priv = ws_private * PAGESIZE_KB;
   ws_usage->shareable = ws_shareable * PAGESIZE_KB;
   ws_usage->shared = ws_shared * PAGESIZE_KB;
+
   return true;
 }
 
+// This function calculates the proportional set size for a process.
+bool ProcessMetrics::GetProportionalSetSizeBytes(uint64_t* pss_bytes) const {
+  double ws_pss = 0.0f;
+
+  WorkingSetInformationBuffer buffer;
+  if (!buffer.GetPageEntries(process_))
+    return false;
+
+  for (unsigned int i = 0; i < buffer.GetPageEntriesNumber(); i++) {
+    if (buffer->WorkingSetInfo[i].Shared &&
+        buffer->WorkingSetInfo[i].ShareCount > 0)
+      ws_pss += 1.0f / buffer->WorkingSetInfo[i].ShareCount;

[brucedawson, 2016/12/06 02:58:55]

Remove trailing 'f' from this line and line 264. The 'f' on this line may
actively harm precision by triggering float-precision division instead of
double-precision.

[chengx, 2016/12/06 19:40:09]

Done.

+    else
+      ws_pss += 1.0f;
+  }
+
+  *pss_bytes = static_cast<uint64_t>(ws_pss);

[Primiano Tucci (use gerrit), 2016/12/06 17:47:56]

1) isn't this missing a * 4096 ? As the name suggests these are bytes, but you
are adding 1 (or a fraction) .
2) Are all pages 4096 bytes on Windows?

[chengx, 2016/12/06 19:40:09]

Yes, you are right. I am missing a *4096, which is the page size in Windows.
Thanks!

+  return true;
+}
+
 static uint64_t FileTimeToUTC(const FILETIME& ftime) {
   LARGE_INTEGER li;
   li.LowPart = ftime.dwLowDateTime;
   li.HighPart = ftime.dwHighDateTime;
   return li.QuadPart;
 }
 
 double ProcessMetrics::GetCPUUsage() {
   FILETIME creation_time;
   FILETIME exit_time;
@@ skipping 73 matching lines @@
 
   meminfo->total = mem_status.ullTotalPhys / 1024;
   meminfo->free = mem_status.ullAvailPhys / 1024;
   meminfo->swap_total = mem_status.ullTotalPageFile / 1024;
   meminfo->swap_free = mem_status.ullAvailPageFile / 1024;
 
   return true;
 }
 
 }  // namespace base