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

Distinguish (program) and (idle) in the time profiles #38

Closed
aalexand opened this issue Nov 1, 2017 · 10 comments
Closed

Distinguish (program) and (idle) in the time profiles #38

aalexand opened this issue Nov 1, 2017 · 10 comments
Assignees
Labels
api: cloudprofiler Issues related to the googleapis/cloud-profiler-nodejs API. 🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@aalexand
Copy link
Contributor

aalexand commented Nov 1, 2017

@ofrobots @nolanmar511

Currently the time profiles contain a pseudo-node named "(program)" for samples which do not have any active Node.js code running. I find this name misleading. It would be clearer if it would be named "(idle)".

@aalexand aalexand changed the title Consider renaming (program) to (idle) in the time profiles Rename (program) to (idle) in the time profiles Nov 19, 2017
@aalexand
Copy link
Contributor Author

aalexand commented Dec 6, 2017

We should confirm that (program) is used when and only when the Node.js thread is idle. E.g. that there are no other usages of this term for some additional rare states of the thread.

@aalexand
Copy link
Contributor Author

aalexand commented Dec 9, 2017

@nolanmar511 Did we say during the discussion last time that you are going to look at the profiler source code to confirm that (program) only has the (idle) meaning?

@nolanmar511
Copy link
Contributor

Yes, we did say that -- I do need to look at the source code to confirm that (program) only has the (idle) meaning. Thank you for reminding me of this.

@nolanmar511
Copy link
Contributor

It does look like the v8 differentiates between "(program)" and "(idle)", given that both strings are defined here and idle and program appear to be used differently here.

I've not yet found any precise documentation on what "(program)" includes, but this Stack Overflow question referenced this bug from 2012, which suggests "(program)" (at one point and when using --inspect) referenced a mix of things, including idle time and native code execution.

Although there seems to be some concept of "(idle)" in v8-profilers, I've not seen a profile with that string. So, I'm guessing that idle time is encompassed in "(program)", but I'm don't think everything in program is idle.

@aalexand
Copy link
Contributor Author

Closing this for now. We'll just use the naming the runtime uses, without any modifications.

@aalexand aalexand reopened this Feb 13, 2018
@aalexand
Copy link
Contributor Author

Re-opening to take a look and at least document somewhere the exact meaning of (idle) and (program).

@aalexand aalexand assigned aalexand and unassigned nolanmar511 Feb 13, 2018
@aalexand aalexand assigned ofrobots and unassigned aalexand May 8, 2018
@aalexand
Copy link
Contributor Author

aalexand commented May 8, 2018

/cc @ofrobots

@ofrobots
Copy link
Contributor

ofrobots commented May 8, 2018

notes from discussion: It might be possible to distinguish real idle times vs. Node.js doing stuff – it might worth looking at how Chrome uses SetIdle API from V8 and whether something similar can be done to distinguish idle from program.

@aalexand
Copy link
Contributor Author

aalexand commented Jun 5, 2018

@ofrobots Any update on this? It might make sense to at least do the simple thing first and make sure that all of the time where the main thread is not executing any JS code is marked as (idle) rather than (program). Reporting everything as (program) doesn't make much sense.

@ofrobots ofrobots changed the title Rename (program) to (idle) in the time profiles Distinguish (program) and (idle) in the time profiles Jul 6, 2018
@ofrobots
Copy link
Contributor

ofrobots commented Jul 6, 2018

I started looking at this. It turns out that Node is not reporting true idle times to the VM by default. See nodejs/node#19009 (comment). A real fix for this would be for Node to report these phases by default, but we can work-around this in the meanwhile. WIP PR forthcoming.

ofrobots added a commit to ofrobots/cloud-profiler-nodejs that referenced this issue Jul 6, 2018
aalexand pushed a commit that referenced this issue Jul 17, 2018
fix: distinguish idle from program time

Fixes: #38
@google-cloud-label-sync google-cloud-label-sync bot added the api: cloudprofiler Issues related to the googleapis/cloud-profiler-nodejs API. label Jan 31, 2020
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudprofiler Issues related to the googleapis/cloud-profiler-nodejs API. 🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

4 participants