Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement process.getProcessMemoryInfo to get the process memory info #16591

Merged
merged 1 commit into from Jan 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 73 additions & 1 deletion atom/common/api/atom_bindings.cc
Expand Up @@ -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/atom_version.h"
#include "atom/common/chrome_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 "brightray/common/application_info.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 {

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -209,6 +220,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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Memory Info is available only after app ready");
"Memory information is only available after the app's 'ready' event (See: electronjs.org/docs/api/app)");

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) {
Expand Down
16 changes: 16 additions & 0 deletions atom/common/api/atom_bindings.h
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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_;
Expand Down
22 changes: 22 additions & 0 deletions docs/api/process.md
Expand Up @@ -14,6 +14,7 @@ In sandboxed renderers the `process` object contains only a subset of the APIs:
- `crash()`
- `hang()`
- `getHeapStatistics()`
- `getProcessMemoryInfo()`
- `getSystemMemoryInfo()`
- `getCPUUsage()`
- `getIOCounters()`
Expand Down Expand Up @@ -157,6 +158,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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
that all statistics are reported in Kilobytes.
that all statistics are reported in kilobytes.

(Typically spelled using lowercase)

This api should be called after app ready.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This api should be called after app ready.
This API should not be called before the `app` has emitted the `ready` event.


Chromium does not provide `residentSet` value for macOS. This is because macOS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Chromium does not provide `residentSet` value for macOS. This is because macOS
Chromium does not provide a `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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result the resident set size value is not what one would expect. `private` memory
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`:
Expand Down
22 changes: 12 additions & 10 deletions spec/api-process-spec.js
Expand Up @@ -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', () => {
Expand Down