Repository navigation
proposal: add num cpu cores to process.report output #307
Description
In the report generated by process.report, we have resourceUsage.cpuConsumptionPercent.
As per the original PR, this is the percentage of CPU in use across all cpu cores. The issue I'm raising here didn't seem to be considered--rather the maintainer decided that granular information about each core was not useful (and they are right).
Now, it's been awhile since my computer architecture classes, but what I think this means is that you cannot just look at the number reported and know what it means. A usage of 100% on system A may be a problem, but a usage of 120% on system B might not be.
(We are not guaranteed to be consuming the report on the machine it was generated on, either.)
So, I propose adding header.osCpus, which would be a number corresponding to the output of os.cpus().length. A consumer can then use both of these to determine the theoretical ceiling of the CPU consumption percentage value (multiply header.osCpus by 100).
If it's a good idea, I could implement. If I can do it in lib/internal/process/report.js that'd be even better. 😝
Activity
sam-github commentedon Jun 10, 2019
Seems pretty reasonable to me, @richardlau, thoughts?
richardlau commentedon Jun 10, 2019
Yes, sounds reasonable. One question is whether it would be useful to include more information about the CPU's in the report, e.g. the information returned by
uv_cpu_info()(i.e. the contents of the returneduv_cpu_info_t's)?The report is written out in C++ code -- for the
headersection see: https://github.com/nodejs/node/blob/04633eeeb93c5a86a6635940eb4f34a7839ada11/src/node_report.cc#L150-L236boneskull commentedon Jun 11, 2019
@richardlau Isn't the cpu time information in the
uv_cpu_info_tput intoresourceUsage.userCpuSeconds&resourceUsage.kernelCpuSeconds? So it'd be essentially duplicate information?At any rate, I don't have a use for anything past the number of cores, and could only speculate on what someone would want to do with the extra info here.
richardlau commentedon Jun 11, 2019
resourceUsageis obtained fromuv_getrusage(): https://github.com/nodejs/node/blob/155caf72bd7972aa46a7b4e8210d13150f8feb43/src/node_report.cc#L423-L452It may well be that the information is duplicated in the two calls.
The only reason I'm speculating on whether it's worth including the extra information is in terms of the report format (i.e. we could go the simple route and have
header.osCpusas the count but then if we wanted to expand the information that would potentially be a format change).mhdawson commentedon Jun 11, 2019
+1 on adding the number of cores to the report.
report: add cpu info to report output
report: add cpu info to report output
report: add cpu info to report output
mmarchini commentedon Jul 8, 2019
@boneskull nodejs/node#28188 landed, so the number of CPUs can be determined by the number of entries in
osCpus. Can we close this issue?boneskull commentedon Jul 8, 2019
yep!