-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Conversation
Reduce the overhead of the CPU profiler by suppressing SIGPROF signals when sleeping / polling for events. Avoids unnecessary wakeups when the CPU profiler is active. Depends on libuv/libuv#15. Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
Ping. #8849 landed, this is good to go. |
Reduce the overhead of the CPU profiler by suppressing SIGPROF signals when sleeping / polling for events. Avoids unnecessary wakeups when the CPU profiler is active. Depends on https://github.com/libuv/libuv#15. Ref: strongloop/strong-agent#3 PR-URL: #8791 Reviewed-by: Trevor Norris <[email protected]>
Thanks. Landed in 1eb1e0a. |
@bnoordhuis Reverted in 1eb1e0a. Causing massive test failures on my box. |
@trevnorris Can you go into details? Tests pass for me locally on Linux and OS X and I don't see why they wouldn't. I did notice that simple/test-http-many-keep-alive-connections fails on the v0.10 branch but that's unrelated to this change. |
@bnoordhuis Almost every test aborts. Here's a core file for Linux x64: https://cloudup.com/files/icrntqpixKc/download |
@trevnorris That site is timing out for me but I can't do much with just a core file anyway. Can you post the output of |
Quick summary: It's abort'ing in
The full output you requested:
|
Oh, that's interesting. Can you strace it? I wonder if you are getting ENOSYS or EINVAL. |
@bnoordhuis |
@trevnorris Is that the call to epoll_pwait()? Can you post the output of #include <signal.h>
#include <sys/epoll.h>
int main(void) {
struct epoll_event ev;
sigset_t set;
sigfillset(&set);
epoll_pwait(epoll_create1(0), &ev, 1, 1, &set);
return 0;
} |
@bnoordhuis, shouldn't the call to |
@saghul I don't think it's necessary. Fedora upgrades libuv whenever a new version comes out; I image Debian does something similar. And if not, it's easily fixed, by us or by them. |
@bnoordhuis no problem then! :-) |
@bnoordhuis Results from your code:
Results from node:
|
Bizarre... the more so because the first epoll_pwait() call succeeds. The only thing I can think of is that fd 5 ceases to be an epoll file descriptor between the two calls. @trevnorris Is your strace new enough that it understands the -y switch? |
@bnoordhuis Yeah. Here's the same command, but also with
|
Here's some additional output:
|
@trevnorris Thanks! I think I know what the issue is. Can you try libuv/libuv#83 and see if that improves matters? |
Reduce the overhead of the CPU profiler by suppressing SIGPROF signals when sleeping / polling for events. Avoids unnecessary wakeups when the CPU profiler is active. Depends on https://github.com/libuv/libuv#15. Ref: strongloop/strong-agent#3 PR-URL: nodejs#8791 Reviewed-by: Trevor Norris <[email protected]>
@bnoordhuis Side note, libuv/libuv#83 landed on the v1.x branch, but this patch is for v0.10. I'll see if I can manually apply the patch to the v0.10 branch of libuv. |
@trevnorris it also landed in 0.10: libuv/libuv@fa0b08f |
@saghul ah, thanks. :) |
@bnoordhuis that seems to fix the problem. @saghul will there be another v0.10 libuv tag? If not I'll just update |
Sure, we can make a new release. I'll see if I can do it tomorrow.
|
@trevnorris @bnoordhuis ... is there reason to keep this one open still? |
I'm going to close this. I still think this is a good change but it needs an update to libuv v0.10.37 and the PR as-is breaks v8-profiler (see nodejs/node#2324). The fix would be trivial if v0.10 had the command line parsing infrastructure that v4 has but it doesn't and porting that over is too much work and risk. |
Reduce the overhead of the CPU profiler by suppressing SIGPROF signals
when sleeping / polling for events. Avoids unnecessary wakeups when
the CPU profiler is active. Depends on libuv/libuv#15.
Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
See also #8789.
R=@trevnorris?