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

Fetch Process resource usage from WebContents #5413

Merged
merged 12 commits into from May 14, 2016

Conversation

Projects
None yet
2 participants
@paulcbetts
Contributor

paulcbetts commented May 6, 2016

This PR adds a new method to WebFrame, getResourceUsage - it returns more detailed memory usage information than performance.memory for a given renderer process:

import {webFrame} from 'electron';
let usage = webFrame.getResourceUsage();

This PR also adds a purgeCaches method that attempts to free memory that is no longer being used (i.e. images from a previous navigation, etc etc):

import {webFrame} from 'electron';
let usage1 = webFrame.getResourceUsage();

webFrame.purgeCaches();

let usage2 = webFrame.getResourceUsage();

console.log(`Before: ${JSON.stringify(usage1)}`);
console.log(`After: ${JSON.stringify(usage2)}`);

Note that blindly calling this method probably makes Electron slower since it will have to refill these emptied caches, you should only call it if an event in your app has occured that makes you think your page is actually using less memory (i.e. you have navigated from a super heavy page to a mostly empty one, and intend to stay there)

@@ -20,5 +20,6 @@ exports.load = function (appUrl) {
})
mainWindow.loadURL(appUrl)
mainWindow.focus()
mainWindow.webContents.getResourceUsage((x) => console.log(x));

This comment has been minimized.

@paulcbetts

paulcbetts May 6, 2016

Contributor

This call seems to always fail with "Insufficient number of arguments" - any idea what I'm doing wrong with marshaling this @zcbenz?

@@ -1266,7 +1273,8 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
.SetProperty("session", &WebContents::Session)
.SetProperty("hostWebContents", &WebContents::HostWebContents)
.SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents)
.SetProperty("debugger", &WebContents::Debugger);
.SetProperty("debugger", &WebContents::Debugger)
.SetProperty("getResourceUsage", &WebContents::GetResourceUsage);

This comment has been minimized.

@zcbenz

zcbenz May 7, 2016

Contributor

Should be SetMethod.

@@ -0,0 +1,39 @@
// Copyright 2015 The Chromium Authors. All rights reserved.

This comment has been minimized.

@zcbenz

zcbenz May 7, 2016

Contributor

This file is not needed.

This comment has been minimized.

@paulcbetts

paulcbetts May 9, 2016

Contributor

I wanted to keep it for posterity and in case we get the ability to build mojo bindings directly in the future (which we'll probably need because Chromium itself is moving towards making everything based on mojo), but I can 🔥 it for now

This comment has been minimized.

@zcbenz

zcbenz May 10, 2016

Contributor

That makes sense, let's keep it then.

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented May 12, 2016

I think we don't actually need to use the ResourceUsageReporter service, it only provides two information: 1) V8 memory; 2) blink cache usage. The former can get received with Node's v8.getHeapStatistics(), the latter can be easily provided by wring a binding to blink::WebCache.

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented May 12, 2016

The former can get received with Node's v8.getHeapStatistics(), the latter can be easily provided by wring a binding to blink::WebCache.

Getting out of the mojo game would be very good because it seems to be a huge pain, lemme try to go down that route

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented May 12, 2016

@zcbenz Much better! This is looking ready to go now

mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate);
dict.Set("count", (uint32_t)stat.count);
dict.Set("size", (uint32_t)(stat.size >> 10));

This comment has been minimized.

@zcbenz

zcbenz May 13, 2016

Contributor

I think most APIs return size in bytes instead of kb.

This comment has been minimized.

@paulcbetts

paulcbetts May 13, 2016

Contributor

I was worried in apps that use way too much memory, that we'd start seeing rounding errors in JavaScript with uint64_t, I saw that mentioned in the native_mate code. If you don't think it's an issue I can flip it back to bytes

This comment has been minimized.

@zcbenz

zcbenz May 13, 2016

Contributor

We can probably convert it to double, so the worst case is we lose some precision.

@@ -168,6 +171,21 @@ mate::Handle<WebFrame> WebFrame::Create(v8::Isolate* isolate) {
return mate::CreateHandle(isolate, new WebFrame(isolate));
}
v8::Local<v8::Value> WebFrame::GetResourceUsage(v8::Isolate* isolate) {

This comment has been minimized.

@zcbenz

zcbenz May 13, 2016

Contributor

You can just return blink::WebCache::ResourceTypeStats in this function, it will be converted automatically.

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented May 13, 2016

There are a few cpplint warnings, mostly about line ending with white space.

@zcbenz zcbenz merged commit 5ec2e8d into master May 14, 2016

8 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
electron-linux-arm Build #3233066 succeeded in 35s
Details
electron-linux-ia32 Build #3233067 succeeded in 33s
Details
electron-linux-x64 Build #3233068 succeeded in 109s
Details
electron-mas-x64 Build #1171 succeeded in 4 min 57 sec
Details
electron-osx-x64 Build #1173 succeeded in 5 min 17 sec
Details
electron-win-ia32 Build #182 succeeded in 6 min 5 sec
Details
electron-win-x64 Build #177 succeeded in 6 min 8 sec
Details

@zcbenz zcbenz deleted the process-resource-usage branch May 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment