From 3deea01d777ca5b2b3a12c33c33600cb679cb72f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 30 Dec 2019 09:33:10 -0500 Subject: [PATCH 1/3] Call uni::AdjustAmountOfExternalAllocatedMemory less often. Apparently calling v8::Isolate::AdjustAmountOfExternalAllocatedMemory frequently results in lots of wasted CPU cycles on garbage collection, per discussion here: https://github.com/meteor/meteor/pull/10527#issuecomment-567982128 This fix was inspired by https://github.com/marudor/libxmljs2/pull/22, which seems to have addressed https://github.com/nodejs/node/issues/30995. Another project that benefitted from adjusting external allocated memory less often: https://github.com/mapnik/node-mapnik/issues/136 --- src/fibers.cc | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/src/fibers.cc b/src/fibers.cc index 011260d..f406a7d 100644 --- a/src/fibers.cc +++ b/src/fibers.cc @@ -412,6 +412,13 @@ class Fiber { static vector orphaned_fibers; static Persistent fatal_stack; + static size_t external_bytes_used; + static size_t external_bytes_reported; + // We report external memory usage only when the difference + // between used and reported exceeds threshold, to avoid calling + // uni::AdjustAmountOfExternalAllocatedMemory too often. + static int external_bytes_threshold; + Isolate* isolate; Persistent handle; Persistent cb; @@ -578,7 +585,7 @@ class Fiber { THROW(Exception::RangeError, "Out of memory"); } that.started = true; - uni::AdjustAmountOfExternalAllocatedMemory(that.isolate, that.this_fiber->size() * GC_ADJUST); + AdjustExternalMemoryOnFiberStart(that); } else { // If the fiber is currently running put the first parameter to `run()` on `yielded`, then // the pending call to `yield()` will return that value. `yielded` in this case is just a @@ -760,7 +767,7 @@ class Fiber { // Do not invoke the garbage collector if there's no context on the stack. It will seg fault // otherwise. - uni::AdjustAmountOfExternalAllocatedMemory(that.isolate, -(int)(that.this_fiber->size() * GC_ADJUST)); + AdjustExternalMemoryOnFiberExit(that); // Don't make weak until after notifying the garbage collector. Otherwise it may try and // free this very fiber! @@ -859,6 +866,24 @@ class Fiber { return uni::Return(uni::NewNumber(Isolate::GetCurrent(), Coroutine::coroutines_created()), info); } + static void AdjustExternalMemoryOnFiberStart(const Fiber& fiber) { + external_bytes_used += fiber.this_fiber->size() * GC_ADJUST; + int diff = external_bytes_used - external_bytes_reported; + if (diff > external_bytes_threshold) { + uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); + external_bytes_reported = external_bytes_used; + } + } + + static void AdjustExternalMemoryOnFiberExit(const Fiber& fiber) { + external_bytes_used -= fiber.this_fiber->size() * GC_ADJUST; + int diff = external_bytes_used - external_bytes_reported; + if (-diff > external_bytes_threshold) { + uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); + external_bytes_reported = external_bytes_used; + } + } + public: /** * Initialize the Fiber library. @@ -918,6 +943,14 @@ Locker* Fiber::global_locker; Fiber* Fiber::current = NULL; vector Fiber::orphaned_fibers; Persistent Fiber::fatal_stack; + +// Used by AdjustExternalMemoryOnFiber{Start,Exit} to avoid calling +// uni::AdjustAmountOfExternalAllocatedMemory every time a Fiber starts +// or exits. +size_t Fiber::external_bytes_used = 0; +size_t Fiber::external_bytes_reported = 0; +int Fiber::external_bytes_threshold = 1024 * 1024; + bool did_init = false; #if !NODE_VERSION_AT_LEAST(0,10,0) From df2c3f135f0f21bfd799eb7f8d33740c0cee6688 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 30 Dec 2019 11:33:06 -0500 Subject: [PATCH 2/3] Compute adjustment threshold using multiple of Fiber size. Thanks to the allocation of the Coroutine stack (see set_stack_size), each Fiber has a size of approximately 1MB on a 64-bit machine, so using a threshold of 1MB gives no improvement over adjusting the memory every single time a fiber starts or exits. Instead (with this commit), we adjust memory whenever the gap exceeds 10x the size of a Fiber. --- src/fibers.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/fibers.cc b/src/fibers.cc index f406a7d..998985d 100644 --- a/src/fibers.cc +++ b/src/fibers.cc @@ -415,9 +415,9 @@ class Fiber { static size_t external_bytes_used; static size_t external_bytes_reported; // We report external memory usage only when the difference - // between used and reported exceeds threshold, to avoid calling + // between used and reported exceeds a threshold, to avoid calling // uni::AdjustAmountOfExternalAllocatedMemory too often. - static int external_bytes_threshold; + static int external_bytes_threshold_factor; Isolate* isolate; Persistent handle; @@ -867,18 +867,20 @@ class Fiber { } static void AdjustExternalMemoryOnFiberStart(const Fiber& fiber) { - external_bytes_used += fiber.this_fiber->size() * GC_ADJUST; + int size = fiber.this_fiber->size(); + external_bytes_used += size * GC_ADJUST; int diff = external_bytes_used - external_bytes_reported; - if (diff > external_bytes_threshold) { + if (diff > size * external_bytes_threshold_factor) { uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); external_bytes_reported = external_bytes_used; } } static void AdjustExternalMemoryOnFiberExit(const Fiber& fiber) { - external_bytes_used -= fiber.this_fiber->size() * GC_ADJUST; + int size = fiber.this_fiber->size(); + external_bytes_used -= size * GC_ADJUST; int diff = external_bytes_used - external_bytes_reported; - if (-diff > external_bytes_threshold) { + if (-diff > size * external_bytes_threshold_factor) { uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); external_bytes_reported = external_bytes_used; } @@ -949,7 +951,7 @@ Persistent Fiber::fatal_stack; // or exits. size_t Fiber::external_bytes_used = 0; size_t Fiber::external_bytes_reported = 0; -int Fiber::external_bytes_threshold = 1024 * 1024; +int Fiber::external_bytes_threshold_factor = 10; bool did_init = false; From 4e30aed9aba380a9f11cb6f1c51ebf535bd2875a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 2 Jan 2020 10:43:50 -0500 Subject: [PATCH 3/3] Stop using AdjustAmountOfExternalAllocatedMemory entirely. https://github.com/laverdet/node-fibers/pull/429#issuecomment-569979499 --- src/fibers.cc | 47 ----------------------------------------------- 1 file changed, 47 deletions(-) diff --git a/src/fibers.cc b/src/fibers.cc index 998985d..e26b216 100644 --- a/src/fibers.cc +++ b/src/fibers.cc @@ -9,13 +9,6 @@ #define THROW(x, m) return uni::Return(uni::ThrowException(Isolate::GetCurrent(), x(uni::NewLatin1String(Isolate::GetCurrent(), m))), args) -// Run GC more often when debugging -#ifdef DEBUG -#define GC_ADJUST 100 -#else -#define GC_ADJUST 1 -#endif - using namespace std; using namespace v8; @@ -412,13 +405,6 @@ class Fiber { static vector orphaned_fibers; static Persistent fatal_stack; - static size_t external_bytes_used; - static size_t external_bytes_reported; - // We report external memory usage only when the difference - // between used and reported exceeds a threshold, to avoid calling - // uni::AdjustAmountOfExternalAllocatedMemory too often. - static int external_bytes_threshold_factor; - Isolate* isolate; Persistent handle; Persistent cb; @@ -585,7 +571,6 @@ class Fiber { THROW(Exception::RangeError, "Out of memory"); } that.started = true; - AdjustExternalMemoryOnFiberStart(that); } else { // If the fiber is currently running put the first parameter to `run()` on `yielded`, then // the pending call to `yield()` will return that value. `yielded` in this case is just a @@ -765,10 +750,6 @@ class Fiber { that.yielded_exception = false; } - // Do not invoke the garbage collector if there's no context on the stack. It will seg fault - // otherwise. - AdjustExternalMemoryOnFiberExit(that); - // Don't make weak until after notifying the garbage collector. Otherwise it may try and // free this very fiber! if (!that.zombie) { @@ -866,26 +847,6 @@ class Fiber { return uni::Return(uni::NewNumber(Isolate::GetCurrent(), Coroutine::coroutines_created()), info); } - static void AdjustExternalMemoryOnFiberStart(const Fiber& fiber) { - int size = fiber.this_fiber->size(); - external_bytes_used += size * GC_ADJUST; - int diff = external_bytes_used - external_bytes_reported; - if (diff > size * external_bytes_threshold_factor) { - uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); - external_bytes_reported = external_bytes_used; - } - } - - static void AdjustExternalMemoryOnFiberExit(const Fiber& fiber) { - int size = fiber.this_fiber->size(); - external_bytes_used -= size * GC_ADJUST; - int diff = external_bytes_used - external_bytes_reported; - if (-diff > size * external_bytes_threshold_factor) { - uni::AdjustAmountOfExternalAllocatedMemory(fiber.isolate, diff); - external_bytes_reported = external_bytes_used; - } - } - public: /** * Initialize the Fiber library. @@ -945,14 +906,6 @@ Locker* Fiber::global_locker; Fiber* Fiber::current = NULL; vector Fiber::orphaned_fibers; Persistent Fiber::fatal_stack; - -// Used by AdjustExternalMemoryOnFiber{Start,Exit} to avoid calling -// uni::AdjustAmountOfExternalAllocatedMemory every time a Fiber starts -// or exits. -size_t Fiber::external_bytes_used = 0; -size_t Fiber::external_bytes_reported = 0; -int Fiber::external_bytes_threshold_factor = 10; - bool did_init = false; #if !NODE_VERSION_AT_LEAST(0,10,0)