Add function to compute proportional set size for OS_WIN

base/process/process_metrics_win.cc

Index: base/process/process_metrics_win.cc
diff --git a/base/process/process_metrics_win.cc b/base/process/process_metrics_win.cc
index 5b2777bb364bfa1c53679432fd511acfb8dbd4a4..8bff6c0f536d90593bb53ad644b2d800534a30eb 100644
--- a/base/process/process_metrics_win.cc
+++ b/base/process/process_metrics_win.cc
@@ -145,7 +145,7 @@ namespace {
 
 class WorkingSetInformationBuffer {
  public:
-  WorkingSetInformationBuffer() {}
+  WorkingSetInformationBuffer() { number_of_entries = 0; }
   ~WorkingSetInformationBuffer() { Clear(); }
 
   bool Reserve(size_t size) {
@@ -155,9 +155,54 @@ class WorkingSetInformationBuffer {
     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_);
@@ -167,6 +212,9 @@ class WorkingSetInformationBuffer {
   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
@@ -179,44 +227,11 @@ bool ProcessMetrics::GetWorkingSetKBytes(WorkingSetKBytes* ws_usage) const {
   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.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;
-    }
-  }
+  if (!buffer.GetPageEntries(process_))
+    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)
@@ -229,6 +244,27 @@ bool ProcessMetrics::GetWorkingSetKBytes(WorkingSetKBytes* ws_usage) const {
   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;
 }