From 6ebd85e10535dfaa9181842fe73834e51d4d3e6c Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 27 Nov 2014 07:15:54 +0100 Subject: [PATCH 1/2] v8: don't busy loop in cpu profiler thread Reduce the overhead of the CPU profiler by replacing 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. Before this commit, the thread would effectively busy loop and consume 100% CPU time. By forcing a one nanosecond sleep period rounded up to the task scheduler's granularity (about 50 us on Linux), CPU usage for the processor thread now hovers around 10-20% for a busy application. PR-URL: https://github.com/joyent/node/pull/8789 Ref: https://github.com/strongloop/strong-agent/issues/3 Reviewed-by: Trevor Norris --- deps/v8/src/platform-freebsd.cc | 5 ----- deps/v8/src/platform-linux.cc | 5 ----- deps/v8/src/platform-macos.cc | 5 ----- deps/v8/src/platform-openbsd.cc | 5 ----- deps/v8/src/platform-posix.cc | 6 ++++++ deps/v8/src/platform-solaris.cc | 5 ----- deps/v8/tools/gyp/v8.gyp | 2 +- 7 files changed, 7 insertions(+), 26 deletions(-) diff --git a/deps/v8/src/platform-freebsd.cc b/deps/v8/src/platform-freebsd.cc index 511759c485e..5c90c6bdae8 100644 --- a/deps/v8/src/platform-freebsd.cc +++ b/deps/v8/src/platform-freebsd.cc @@ -539,11 +539,6 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) { } -void Thread::YieldCPU() { - sched_yield(); -} - - class FreeBSDMutex : public Mutex { public: FreeBSDMutex() { diff --git a/deps/v8/src/platform-linux.cc b/deps/v8/src/platform-linux.cc index beb2ccee297..3d6b3044e7b 100644 --- a/deps/v8/src/platform-linux.cc +++ b/deps/v8/src/platform-linux.cc @@ -812,11 +812,6 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) { } -void Thread::YieldCPU() { - sched_yield(); -} - - class LinuxMutex : public Mutex { public: LinuxMutex() { diff --git a/deps/v8/src/platform-macos.cc b/deps/v8/src/platform-macos.cc index a216f6e4cac..e54e3e4a421 100644 --- a/deps/v8/src/platform-macos.cc +++ b/deps/v8/src/platform-macos.cc @@ -640,11 +640,6 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) { } -void Thread::YieldCPU() { - sched_yield(); -} - - class MacOSMutex : public Mutex { public: MacOSMutex() { diff --git a/deps/v8/src/platform-openbsd.cc b/deps/v8/src/platform-openbsd.cc index 408d4dc0f84..72167de9202 100644 --- a/deps/v8/src/platform-openbsd.cc +++ b/deps/v8/src/platform-openbsd.cc @@ -593,11 +593,6 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) { } -void Thread::YieldCPU() { - sched_yield(); -} - - class OpenBSDMutex : public Mutex { public: OpenBSDMutex() { diff --git a/deps/v8/src/platform-posix.cc b/deps/v8/src/platform-posix.cc index 5c3529d4458..8aecd560e90 100644 --- a/deps/v8/src/platform-posix.cc +++ b/deps/v8/src/platform-posix.cc @@ -392,6 +392,12 @@ void OS::StrNCpy(Vector dest, const char* src, size_t n) { } +void Thread::YieldCPU() { + const timespec delay = { 0, 1 }; + nanosleep(&delay, NULL); +} + + // ---------------------------------------------------------------------------- // POSIX socket support. // diff --git a/deps/v8/src/platform-solaris.cc b/deps/v8/src/platform-solaris.cc index 07718fe50b9..4e95ecc6a48 100644 --- a/deps/v8/src/platform-solaris.cc +++ b/deps/v8/src/platform-solaris.cc @@ -527,11 +527,6 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) { } -void Thread::YieldCPU() { - sched_yield(); -} - - class SolarisMutex : public Mutex { public: SolarisMutex() { diff --git a/deps/v8/tools/gyp/v8.gyp b/deps/v8/tools/gyp/v8.gyp index 71cf36649ad..c304925908a 100644 --- a/deps/v8/tools/gyp/v8.gyp +++ b/deps/v8/tools/gyp/v8.gyp @@ -715,7 +715,7 @@ ['OS=="solaris"', { 'link_settings': { 'libraries': [ - '-lsocket -lnsl', + '-lsocket -lnsl -lrt', ]}, 'sources': [ '../../src/platform-solaris.cc', From 31051e5c7b253b6a0006b536188fc0494bd6e082 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Thu, 8 Jan 2015 12:17:54 -0800 Subject: [PATCH 2/2] deps: revert backport b593aa8 from v8 upstream This reverts commit 45f1330425b671905a02fe30ee7a1dd9544c2709. 45f1330425b671905a02fe30ee7a1dd9544c2709 was basically breaking node-inspector. V8 landed a patch upstream that would probably fix these issues (see https://codereview.chromium.org/813873007), but without the ability to properly test it in the wild, it's safer to just revert the breaking change. Fixes #8948. Reviewed-By: Colin Ihrig Reviewed-by: Trevor Norris --- deps/v8/src/debug-debugger.js | 5 ++--- deps/v8/src/mirror-debugger.js | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/deps/v8/src/debug-debugger.js b/deps/v8/src/debug-debugger.js index be39c491b6d..a27961fa460 100644 --- a/deps/v8/src/debug-debugger.js +++ b/deps/v8/src/debug-debugger.js @@ -24,7 +24,6 @@ // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"use strict"; // Default number of frames to include in the response to backtrace request. var kDefaultBacktraceLength = 10; @@ -678,7 +677,7 @@ Debug.setBreakPoint = function(func, opt_line, opt_column, opt_condition) { Debug.setBreakPointByScriptIdAndPosition = function(script_id, position, condition, enabled) { - var break_point = MakeBreakPoint(position); + break_point = MakeBreakPoint(position); break_point.setCondition(condition); if (!enabled) { break_point.disable(); @@ -743,7 +742,7 @@ Debug.clearBreakPoint = function(break_point_number) { Debug.clearAllBreakPoints = function() { for (var i = 0; i < break_points.length; i++) { - var break_point = break_points[i]; + break_point = break_points[i]; %ClearBreakPoint(break_point); } break_points = []; diff --git a/deps/v8/src/mirror-debugger.js b/deps/v8/src/mirror-debugger.js index 6bec59bcb3e..7f1a05aed94 100644 --- a/deps/v8/src/mirror-debugger.js +++ b/deps/v8/src/mirror-debugger.js @@ -24,7 +24,6 @@ // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"use strict"; // Handle id counters. var next_handle_ = 0; @@ -56,7 +55,7 @@ function MakeMirror(value, opt_transient) { // Look for non transient mirrors in the mirror cache. if (!opt_transient) { - for (var id in mirror_cache_) { + for (id in mirror_cache_) { mirror = mirror_cache_[id]; if (mirror.value() === value) { return mirror; @@ -1155,11 +1154,11 @@ ErrorMirror.prototype.toText = function() { // Use the same text representation as in messages.js. var text; try { - text = %_CallFunction(this.value_, builtins.ErrorToString); + str = %_CallFunction(this.value_, builtins.ErrorToString); } catch (e) { - text = '#'; + str = '#'; } - return text; + return str; };