feat: Implement process.getProcessMemoryInfo to get the process memor…

…y usage (electron#14847) * feat: Implement process.getMemoryFootprint to get the process memory usage * Add spec * fix: must enter node env in callback * Update function call * Update spec * Update API data * update spec * Update include * update test for shared bytes * Update atom/common/api/atom_bindings.cc Co-Authored-By: nitsakh <[email protected]> * Update atom/common/api/atom_bindings.cc Co-Authored-By: nitsakh <[email protected]> * Update API * Update the callback isolate * Update to work after app ready * Update docs * Update docs/api/process.md Co-Authored-By: nitsakh <[email protected]> * Update docs/api/process.md Co-Authored-By: nitsakh <[email protected]> * Fix crash
daverodgman · Nov 28, 2018 · 9890d1e · 9890d1e
1 parent f563fc9
commit 9890d1e
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 11 deletions.
diff --git a/atom/common/api/atom_bindings.cc b/atom/common/api/atom_bindings.cc
@@ -7,21 +7,31 @@
 #include <algorithm>
 #include <iostream>
 #include <string>
+#include <utility>
+#include <vector>
 
+#include "atom/browser/browser.h"
 #include "atom/common/api/locker.h"
 #include "atom/common/application_info.h"
 #include "atom/common/atom_version.h"
 #include "atom/common/heap_snapshot.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
-#include "atom/common/node_includes.h"
+#include "atom/common/promise_util.h"
 #include "base/logging.h"
+#include "base/process/process_handle.h"
 #include "base/process/process_info.h"
 #include "base/process/process_metrics_iocounters.h"
 #include "base/sys_info.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/common/chrome_version.h"
 #include "native_mate/dictionary.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/global_memory_dump.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
+
+// Must be the last in the includes list, otherwise the definition of chromium
+// macros conflicts with node macros.
+#include "atom/common/node_includes.h"
 
 namespace atom {
 
@@ -61,6 +71,7 @@ void AtomBindings::BindTo(v8::Isolate* isolate, v8::Local<v8::Object> process) {
   dict.SetMethod("getHeapStatistics", &GetHeapStatistics);
   dict.SetMethod("getCreationTime", &GetCreationTime);
   dict.SetMethod("getSystemMemoryInfo", &GetSystemMemoryInfo);
+  dict.SetMethod("getProcessMemoryInfo", &GetProcessMemoryInfo);
   dict.SetMethod("getCPUUsage", base::Bind(&AtomBindings::GetCPUUsage,
                                            base::Unretained(metrics_.get())));
   dict.SetMethod("getIOCounters", &GetIOCounters);
@@ -208,6 +219,67 @@ v8::Local<v8::Value> AtomBindings::GetSystemMemoryInfo(v8::Isolate* isolate,
   return dict.GetHandle();
 }
 
+// static
+v8::Local<v8::Promise> AtomBindings::GetProcessMemoryInfo(
+    v8::Isolate* isolate) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate);
+
+  if (mate::Locker::IsBrowserProcess() && !Browser::Get()->is_ready()) {
+    promise->RejectWithErrorMessage(
+        "Memory Info is available only after app ready");
+    return promise->GetHandle();
+  }
+
+  v8::Global<v8::Context> context(isolate, isolate->GetCurrentContext());
+  memory_instrumentation::MemoryInstrumentation::GetInstance()
+      ->RequestGlobalDumpForPid(base::GetCurrentProcId(),
+                                std::vector<std::string>(),
+                                base::Bind(&AtomBindings::DidReceiveMemoryDump,
+                                           std::move(context), promise));
+  return promise->GetHandle();
+}
+
+// static
+void AtomBindings::DidReceiveMemoryDump(
+    const v8::Global<v8::Context>& context,
+    scoped_refptr<util::Promise> promise,
+    bool success,
+    std::unique_ptr<memory_instrumentation::GlobalMemoryDump> global_dump) {
+  v8::Isolate* isolate = promise->isolate();
+  mate::Locker locker(isolate);
+  v8::HandleScope handle_scope(isolate);
+  v8::MicrotasksScope script_scope(isolate,
+                                   v8::MicrotasksScope::kRunMicrotasks);
+  v8::Context::Scope context_scope(
+      v8::Local<v8::Context>::New(isolate, context));
+
+  if (!success) {
+    promise->RejectWithErrorMessage("Failed to create memory dump");
+    return;
+  }
+
+  bool resolved = false;
+  for (const memory_instrumentation::GlobalMemoryDump::ProcessDump& dump :
+       global_dump->process_dumps()) {
+    if (base::GetCurrentProcId() == dump.pid()) {
+      mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate);
+      const auto& osdump = dump.os_dump();
+#if defined(OS_LINUX) || defined(OS_WIN)
+      dict.Set("residentSet", osdump.resident_set_kb);
+#endif
+      dict.Set("private", osdump.private_footprint_kb);
+      dict.Set("shared", osdump.shared_footprint_kb);
+      promise->Resolve(dict.GetHandle());
+      resolved = true;
+      break;
+    }
+  }
+  if (!resolved) {
+    promise->RejectWithErrorMessage(
+        R"(Failed to find current process memory details in memory dump)");
+  }
+}
+
 // static
 v8::Local<v8::Value> AtomBindings::GetCPUUsage(base::ProcessMetrics* metrics,
                                                v8::Isolate* isolate) {

diff --git a/atom/common/api/atom_bindings.h b/atom/common/api/atom_bindings.h
@@ -10,18 +10,27 @@
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/string16.h"
 #include "native_mate/arguments.h"
 #include "uv.h"  // NOLINT(build/include)
 #include "v8/include/v8.h"
 
+namespace memory_instrumentation {
+class GlobalMemoryDump;
+}
+
 namespace node {
 class Environment;
 }
 
 namespace atom {
 
+namespace util {
+class Promise;
+}
+
 class AtomBindings {
  public:
   explicit AtomBindings(uv_loop_t* loop);
@@ -41,6 +50,7 @@ class AtomBindings {
   static v8::Local<v8::Value> GetCreationTime(v8::Isolate* isolate);
   static v8::Local<v8::Value> GetSystemMemoryInfo(v8::Isolate* isolate,
                                                   mate::Arguments* args);
+  static v8::Local<v8::Promise> GetProcessMemoryInfo(v8::Isolate* isolate);
   static v8::Local<v8::Value> GetCPUUsage(base::ProcessMetrics* metrics,
                                           v8::Isolate* isolate);
   static v8::Local<v8::Value> GetIOCounters(v8::Isolate* isolate);
@@ -52,6 +62,12 @@ class AtomBindings {
 
   static void OnCallNextTick(uv_async_t* handle);
 
+  static void DidReceiveMemoryDump(
+      const v8::Global<v8::Context>& context,
+      scoped_refptr<util::Promise> promise,
+      bool success,
+      std::unique_ptr<memory_instrumentation::GlobalMemoryDump> dump);
+
   uv_async_t call_next_tick_async_;
   std::list<node::Environment*> pending_next_ticks_;
   std::unique_ptr<base::ProcessMetrics> metrics_;

diff --git a/docs/api/process.md b/docs/api/process.md
@@ -14,6 +14,7 @@ In sandboxed renderers the `process` object contains only a subset of the APIs:
 - `crash()`
 - `hang()`
 - `getHeapStatistics()`
+- `getProcessMemoryInfo()`
 - `getSystemMemoryInfo()`
 - `getCPUUsage()`
 - `getIOCounters()`
@@ -162,6 +163,27 @@ Returns `Object`:
 
 Returns an object with V8 heap statistics. Note that all statistics are reported in Kilobytes.
 
+### `process.getProcessMemoryInfo()`
+
+Returns `Object`:
+
+* `residentSet` Integer _Linux_ and _Windows_ - The amount of memory 
+currently pinned to actual physical RAM in Kilobytes.
+* `private` Integer - The amount of memory not shared by other processes, such as
+  JS heap or HTML content in Kilobytes.
+* `shared` Integer - The amount of memory shared between processes, typically
+  memory consumed by the Electron code itself in Kilobytes.
+
+Returns an object giving memory usage statistics about the current process. Note
+that all statistics are reported in Kilobytes.
+This api should be called after app ready.
+
+Chromium does not provide `residentSet` value for macOS. This is because macOS 
+performs in-memory compression of pages that haven't been recently used. As a
+result the resident set size value is not what one would expect. `private` memory
+is more representative of the actual pre-compression memory usage of the process
+on macOS.
+
 ### `process.getSystemMemoryInfo()`
 
 Returns `Object`:

diff --git a/spec/api-process-spec.js b/spec/api-process-spec.js
@@ -38,16 +38,18 @@ describe('process module', () => {
     })
   })
 
-  // FIXME: Chromium 67 - getProcessMemoryInfo has been removed
-  // describe('process.getProcessMemoryInfo()', () => {
-  //   it('returns process memory info object', () => {
-  //     const processMemoryInfo = process.getProcessMemoryInfo()
-  //     expect(processMemoryInfo.peakWorkingSetSize).to.be.a('number')
-  //     expect(processMemoryInfo.privateBytes).to.be.a('number')
-  //     expect(processMemoryInfo.sharedBytes).to.be.a('number')
-  //     expect(processMemoryInfo.workingSetSize).to.be.a('number')
-  //   })
-  // })
+  describe('process.getProcessMemoryInfo()', async () => {
+    it('resolves promise successfully with valid data', async () => {
+      const memoryInfo = await process.getProcessMemoryInfo()
+      expect(memoryInfo).to.be.an('object')
+      if (process.platform === 'linux' || process.platform === 'windows') {
+        expect(memoryInfo.residentSet).to.be.a('number').greaterThan(0)
+      }
+      expect(memoryInfo.private).to.be.a('number').greaterThan(0)
+      // Shared bytes can be zero
+      expect(memoryInfo.shared).to.be.a('number').greaterThan(-1)
+    })
+  })
 
   describe('process.getSystemMemoryInfo()', () => {
     it('returns system memory info object', () => {