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 137 matching lines @@
   WorkingSetInformationBuffer() {}
   ~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() const { return number_of_entries; }

[stanisc, 2016/12/09 01:19:55]

Nit: Consider renaming this to either GetPageEntryCount or
GetNumberOfPageEntries, or simply GetCount.

[chengx, 2016/12/09 22:34:32]

renamed to GetPageEntryCount

+
+  // This function is used to get page entries for a process.
+  bool GetPageEntries(const ProcessHandle& process_) {

[stanisc, 2016/12/09 01:19:55]

Nit: This doesn't get actual entries so perhaps the name should be different.
How about QueryPageEntries?

[chengx, 2016/12/09 22:34:32]

Done.

+    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

[dcheng, 2016/12/08 20:25:30]

Nit: I know this is just moving code, but this comment feels misleading.

Maybe something like:
On success, |buffer_| is populated with info about the working set of
|process_|. On ERROR_BAD_LENGTH failure, increase the size of the
buffer and try again.

[chengx, 2016/12/08 23:03:13]

That is a good comment and I will take it.

+      if (QueryWorkingSet(process_, buffer_, buffer_size))
+        break;  // Success
+
+      if (GetLastError() != ERROR_BAD_LENGTH)
+        return false;
+
+      number_of_entries = static_cast<DWORD>(buffer_->NumberOfEntries);

[dcheng, 2016/12/08 20:25:30]

Is it possible to just make this a ULONG_PTR as well, so we don't static_cast
all over the place?

[chengx, 2016/12/08 23:03:13]

I personally think ULONG_PTR should be fine. I will include stan, the owner of
this code, in this discussion.

[stanisc, 2016/12/09 01:19:55]

I am not the original author, but I did modify this code recently to fix an OOM
bug. If this is changed to ULONG_PTR that would require using ULONG_PTR
elsewhere where GetPageEntriesNumber is called. How about using a portable type
such as size_t. I think it should be the same as ULONG_PTR?

[chengx, 2016/12/09 22:34:32]

I will just leave the old comment there for now.

+
+      // 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.

[dcheng, 2016/12/08 20:25:30]

I don't think we support win2k?

[stanisc, 2016/12/09 01:19:55]

This might be an old comment.

[chengx, 2016/12/09 22:34:32]

Acknowledged.

+    // 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));
+
+    return true;
+  }
+
  private:
   void Clear() {
     free(buffer_);
     buffer_ = nullptr;
   }
 
   PSAPI_WORKING_SET_INFORMATION* buffer_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(WorkingSetInformationBuffer);

[dcheng, 2016/12/08 20:25:30]

Nit: this is generally last

[chengx, 2016/12/08 23:03:13]

Done.

+
+  // Number of page entries.
+  DWORD number_of_entries = 0;
 };
 
 }  // 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++) {
+  DWORD numPageEntries = buffer.GetPageEntriesNumber();

[dcheng, 2016/12/08 20:25:30]

Nit: num_page_entries

[chengx, 2016/12/08 23:03:13]

Done.

+  for (unsigned int i = 0; i < numPageEntries; i++) {

[dcheng, 2016/12/08 20:25:30]

Match types (this should be DWORD or ULONG_PTR, whatever we end up deciding)

[chengx, 2016/12/08 23:03:13]

I will do DWORD for now.

[stanisc, 2016/12/09 01:19:55]

Consider using size_t as I mentioned above.

[chengx, 2016/12/09 22:34:32]

Done.

     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.0;
+
+  WorkingSetInformationBuffer buffer;
+  if (!buffer.GetPageEntries(process_))
+    return false;
+
+  DWORD numPageEntries = buffer.GetPageEntriesNumber();
+  for (unsigned int i = 0; i < numPageEntries; i++) {

[dcheng, 2016/12/08 20:25:30]

Ditto.

+    if (buffer->WorkingSetInfo[i].Shared &&
+        buffer->WorkingSetInfo[i].ShareCount > 0)
+      ws_pss += 1.0 / buffer->WorkingSetInfo[i].ShareCount;
+    else
+      ws_pss += 1.0;
+  }
+
+  *pss_bytes = static_cast<uint64_t>(ws_pss * GetPageSize());
+  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