-
Notifications
You must be signed in to change notification settings - Fork 78
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
Improve the V8 Profiler performance #444
Comments
Removing from the agenda until we come up with benchmarks for the performance issue. |
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
I've tried to reproduce this behavior but it looks like was fixed on v14 and above. Code: const inspector = require('inspector');
const fs = require('fs');
const session = new inspector.Session();
session.connect();
console.time('enable')
session.post('Profiler.enable', () => {
console.timeEnd('enable')
let a = [];
for (let i = 0; i < 1000000; i++) {
const f = new Function("a", "b", "return a + b");
f(i, i - 1);
a.push(f);
}
console.time('start')
session.post('Profiler.start', () => {
console.timeEnd('start')
console.time('stop')
session.post('Profiler.stop', (err, { profile }) => {
console.timeEnd('stop')
if (!err) {
fs.writeFileSync('./profile.cpuprofile', JSON.stringify(profile));
}
});
});
}); v10 $ node bench-profiler.js
enable: 0.293ms
start: 610.000ms
stop: 235.631ms v12 $ node bench-profiler.js
enable: 0.404ms
start: 640.016ms
stop: 0.473ms v14 $ node bench-profiler.js
enable: 0.682ms
start: 58.032ms
stop: 0.472ms v16 $ node bench-profiler.js
enable: 1.621ms
start: 53.174ms
stop: 0.695ms @Flarna I'm tagging you because I've seen your code example from: #148 (comment) |
We discussed and sounds reasonable to close this issue since the results look good for the production environment. |
Hi, @RafaelGSS, can we reopen this issue? I ran the above bench program in
yet on
I went backwards on 16 LTS, and it's fast - sometimes - on
and it appears always fast, but not as fast as 14 or 17, on
These were all run on a macbook pro 2020, intel. |
Hey! Are you able to send a reproducible code? |
Yes, I ran the exact script referenced above that you posted from the original thread. I did not attempt to repro this on other machines / OS, only on mac (Big Sur). I didn't try on linux, for example. |
Oh, ok. I'll check soon |
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
Recently, I had no time to validate it. Does anyone in @nodejs/diagnostics interested in this discovery? |
If I got it right, we should understand why the profiler takes too much to stop on Node 16.x and 17.x? |
Checking how much time is it taking in all Node.js versions would be a great start. |
Neat! I could give it a try ^^. Feel free to share any resource that could help me. I remember you posted that: /~https://github.com/RafaelGSS/nodejs-bench-operations Could I use a similar approach? |
I think the snippet I sent above would work as well. #444 (comment) |
Hey 👋 Make a simple script and put the results here: /~https://github.com/tony-go/v8-profiler-performance What I did:
I keep the same output but let me know if you prefer a table with average etc... Sorry if it looks a bit bikeshed, but it's my first time ^^ |
Thank you @tony-go. I've run it locally and the max spike I got was 1.5sec ======= iteration number 1 for node v14.19.3 ========
enable: 0.246ms
start: 57.586ms
stop: 0.447ms
======= iteration number 2 for node v14.19.3 ========
enable: 0.25ms
start: 56.623ms
stop: 0.435ms
======= iteration number 3 for node v14.19.3 ========
enable: 0.238ms
start: 58.386ms
stop: 0.445ms
======= iteration number 4 for node v14.19.3 ========
enable: 0.246ms
start: 57.457ms
stop: 0.456ms
======= iteration number 5 for node v14.19.3 ========
enable: 0.267ms
start: 59.748ms
stop: 0.597ms
======= iteration number 6 for node v14.19.3 ========
enable: 0.249ms
start: 56.702ms
stop: 0.444ms
======= iteration number 7 for node v14.19.3 ========
enable: 0.236ms
start: 58.028ms
stop: 0.477ms
======= iteration number 8 for node v14.19.3 ========
enable: 0.245ms
start: 57.485ms
stop: 0.434ms
======= iteration number 9 for node v14.19.3 ========
enable: 0.245ms
start: 57.501ms
stop: 0.455ms
======= iteration number 10 for node v14.19.3 ========
enable: 0.247ms
start: 59.6ms
stop: 0.437ms
There are several variables that might affect this benchmark such as page faults and so on. Nevertheless, I think the results in the latest version are acceptable |
@lastquestion I'll close it. In case you think this need to be fixed, please provide a bit more information of your use case. Thank you! |
Good :)
For my personal culture: if you could share more information regarding stuff I should avoid when I write a benchmark it should be great :) |
As previously mentioned on #148 a lot of work was done on CPU Profiler.
The only tool for profiling NodeJS cross-platform is the usage of
Profiler.start
andProfiler.stop
. However, this dynamic instrumentation blocks the event-loop because it needs to iterate over all functions to map. (CpuProfiler::StartProfiling more specifically) and it is harmful to production environments.This issue is to discuss some ways to improve these usages in the production environment.
The text was updated successfully, but these errors were encountered: