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

Adding CPU & Memory metrics for App #9486

Merged
merged 13 commits into from May 26, 2017

Conversation

Projects
None yet
3 participants
@juturu
Contributor

juturu commented May 16, 2017

  • TODO: Need to deprecate getAppMemoryInfo. What is the process of deprecation?

New API getAppMetrics will return something like below

    {  
      "memory":{  
         "workingSetSize":64920,
         "peakWorkingSetSize":0,
         "privateBytes":21300,
         "sharedBytes":68552
      },
      "cpu":{  
         "percentCPUUsage":0,
         "idleWakeupsPerSecond":0
      },
      "pid":85123,
      "type":"Tab"
   },
   {  
      "memory":{  
         "workingSetSize":78604,
         "peakWorkingSetSize":0,
         "privateBytes":17488,
         "sharedBytes":84520
      },
      "cpu":{  
         "percentCPUUsage":0.40786608393219276,
         "idleWakeupsPerSecond":0
      },
      "pid":85092,
      "type":"GPU"
   },
   {  
      "memory":{  
         "workingSetSize":77032,
         "peakWorkingSetSize":0,
         "privateBytes":29612,
         "sharedBytes":80568
      },
      "cpu":{  
         "percentCPUUsage":0.7511017533585603,
         "idleWakeupsPerSecond":4
      },
      "pid":85091,
      "type":"Browser"
   }
@zeke

This comment has been minimized.

Member

zeke commented May 16, 2017

What is the process of deprecation?

See docs/tutorial/planned-breaking-changes.md

@kevinsawicki

Left a few minor comments, a bunch of the lines look like they could fit on a single line (I left a few comments on specific place) and still be <= 80 characters so maybe take a pass over the diff and re-format the lines that could fit on one line to be one line.

@@ -505,6 +507,19 @@ App::App(v8::Isolate* isolate) {
static_cast<AtomBrowserClient*>(AtomBrowserClient::Get())->set_delegate(this);
Browser::Get()->AddObserver(this);
content::GpuDataManager::GetInstance()->AddObserver(this);
content::BrowserChildProcessObserver::Add(this);
int pid = 0;

This comment has been minimized.

@kevinsawicki

kevinsawicki May 24, 2017

Contributor

I think you can use the chrome helper for this:

ProcessId pid = base::GetCurrentProcId();

Then you won't need the two paths.

@@ -666,6 +682,58 @@ void App::OnGpuProcessCrashed(base::TerminationStatus status) {
status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED);
}
void App::BrowserChildProcessLaunchedAndConnected(
const content::ChildProcessData& data) {
this->ChildProcessLaunched(

This comment has been minimized.

@kevinsawicki

kevinsawicki May 24, 2017

Contributor

It looks like this could all fit on one line.

void App::BrowserChildProcessHostDisconnected(
const content::ChildProcessData& data) {
this->ChildProcessDisconnected(

This comment has been minimized.

@kevinsawicki

kevinsawicki May 24, 2017

Contributor

Could be on the same line.

void App::RenderProcessReady(
content::RenderProcessHost* host) {
this->ChildProcessLaunched(

This comment has been minimized.

@kevinsawicki

kevinsawicki May 24, 2017

Contributor

Could be on the same line I think.

@kevinsawicki kevinsawicki self-assigned this May 25, 2017

kevinsawicki added some commits May 26, 2017

@kevinsawicki kevinsawicki merged commit 9137a22 into master May 26, 2017

0 of 6 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-linux-arm Build #6781109 queued
Details
electron-linux-ia32 Build #6781110 queued
Details
electron-linux-x64 Build #6781111 queued
Details
electron-mas-x64 Build #4333 in progress...
Details

@kevinsawicki kevinsawicki deleted the child-observer branch May 26, 2017

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 26, 2017

Thanks for this @juturu 👍 🚢

@juturu

This comment has been minimized.

Contributor

juturu commented May 26, 2017

Thanks @kevinsawicki 🙇

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