Skip to content

Commit

Permalink
deps: reduce cpu profiler overhead
Browse files Browse the repository at this point in the history
* Block SIGPROF when in the epoll_pwait() system call so the event loop
  doesn't keep waking up on signal delivery.  The clock_gettime() system
  call that libuv does after EINTR is very expensive on virtualized
  systems.

* Replace sched_yield() with nanosleep() in V8's tick event processor
  thread.  The former only yields the CPU when there is another process
  scheduled on the same CPU.

* Fix a bug in the epoll_pwait() system call wrapper in libuv,
  see libuv/libuv#4.

Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
  • Loading branch information
bnoordhuis committed Nov 24, 2014
1 parent 3a08b7c commit 24852b5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 19 deletions.
26 changes: 9 additions & 17 deletions deps/uv/src/unix/linux-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
struct uv__epoll_event e;
ngx_queue_t* q;
uv__io_t* w;
sigset_t set;
uint64_t base;
uint64_t diff;
int nevents;
Expand All @@ -138,7 +139,6 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
int fd;
int op;
int i;
static int no_epoll_wait;

if (loop->nfds == 0) {
assert(ngx_queue_empty(&loop->watcher_queue));
Expand Down Expand Up @@ -184,23 +184,15 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
base = loop->time;
count = 48; /* Benchmarks suggest this gives the best throughput. */

sigemptyset(&set);
sigaddset(&set, SIGPROF);

for (;;) {
if (!no_epoll_wait) {
nfds = uv__epoll_wait(loop->backend_fd,
events,
ARRAY_SIZE(events),
timeout);
if (nfds == -1 && errno == ENOSYS) {
no_epoll_wait = 1;
continue;
}
} else {
nfds = uv__epoll_pwait(loop->backend_fd,
events,
ARRAY_SIZE(events),
timeout,
NULL);
}
nfds = uv__epoll_pwait(loop->backend_fd,
events,
ARRAY_SIZE(events),
timeout,
&set);

/* Update loop->time unconditionally. It's tempting to skip the update when
* timeout == 0 (i.e. non-blocking poll) but there is no guarantee that the
Expand Down
3 changes: 2 additions & 1 deletion deps/uv/src/unix/linux-syscalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "linux-syscalls.h"
#include <unistd.h>
#include <signal.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <errno.h>
Expand Down Expand Up @@ -298,7 +299,7 @@ int uv__epoll_pwait(int epfd,
nevents,
timeout,
sigmask,
sizeof(*sigmask));
_NSIG / sizeof(long));
#else
return errno = ENOSYS, -1;
#endif
Expand Down
4 changes: 3 additions & 1 deletion deps/v8/src/platform-linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <sys/syscall.h>
#include <sys/types.h>
#include <stdlib.h>
#include <time.h>

// Ubuntu Dapper requires memory pages to be marked as
// executable. Otherwise, OS raises an exception when executing code
Expand Down Expand Up @@ -813,7 +814,8 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) {


void Thread::YieldCPU() {
sched_yield();
const timespec delay = { 0, 1 };
nanosleep(&delay, NULL);
}


Expand Down

3 comments on commit 24852b5

@rmg
Copy link

@rmg rmg commented on 24852b5 Nov 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sched_yield to nanosleep change really alters the behaviour of the call.. On a system with load >= nCPU using node clustering, wouldn't this result in a possibly dramatic increase in context switches as the busy processes split their allocated time up across the schedule cycle? Though I suppose that would balance against waiting for the OS to re-schedule the thread...

On second thought, under any sort of load, this would help the node processes get more of their scheduled CPU time by being less polite to the other threads at the same priority.

Of course this is all without actually looking at when/where/why YieldCPU is actually called..

@bnoordhuis
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course this is all without actually looking at when/where/why YieldCPU is actually called..

It's called in cpu-profiler.cc and execution.cc. The call in execution.cc is to implement preemption, a feature that node.js doesn't use and that has been removed in upstream V8.

As you can see, it's basically a busy loop when there are no other processes scheduled on the current CPU. The idea behind this patch is to force the tick processor thread to sleep for the shortest amount of time possible (1 ns, rounded up to whatever granularity the process scheduler has) in order to relax CPU usage.

@rmg
Copy link

@rmg rmg commented on 24852b5 Nov 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Wasn't there some noise a while back when Linux changed the behaviour of sched_yield and how you shouldn't use it for busy loops? Been a long time since I paid attention to LKML and friends..

/me googles.. looks like it was a 2.5 change and was over a decade ago now. Wow.

Please sign in to comment.