New issue
Advanced search Search tips

Issue 7381 link

Starred by 6 users

Issue metadata

Status: Available
Owner:
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: FeatureRequest



Sign in to add a comment

[feature request] mark V8 heap MADV_DONTFORK

Reported by i...@bnoordhuis.nl, Jan 27 2018

Issue description

For fork() performance reasons, node.js would benefit hugely if it could mark the JS heap with `madvise(MADV_DONTFORK)`.

A patch like the one below accomplishes that but adding an embedder callback that is invoked whenever memory is mapped in would also be fine for us.  Happy to do the legwork!

https://github.com/nodejs/node/issues/14917 for context.  Some people apparently have really large JS heaps...

I tried implementing it by parsing /proc/self/maps but it not possible to distinguish between anonymous memory mapped by V8 and, say, memory mapped by a shared library that may need to persist across a fork.


diff --git a/deps/v8/src/base/platform/platform-posix.cc b/deps/v8/src/base/platform/platform-posix.cc
index b873197d3b..6d5fa1b6f6 100644
--- a/deps/v8/src/base/platform/platform-posix.cc
+++ b/deps/v8/src/base/platform/platform-posix.cc
@@ -136,6 +136,9 @@ void* Allocate(void* address, size_t size, OS::MemoryPermission access) {
   void* result =
       mmap(address, actual_size, prot, flags, kMmapFd, kMmapFdOffset);
   if (result == MAP_FAILED) return nullptr;
+#if defined(V8_OS_LINUX)
+  madvise(address, actual_size, MADV_DONTFORK);
+#endif
   return result;
 }
 
Components: API GC
Labels: -Type-Bug Priority-2 Type-FeatureRequest
Status: Available (was: Untriaged)
Please evaluate
This would be a life saver for us <3
Owner: hpayer@chromium.org
Cc: mlippautz@chromium.org u...@chromium.org
I think this looks good from my perspective if node benefits from that. The V8 heap pages should not be used in another process anyway.

Adding other memory people for sanity check.
For sandbox use cases I think we always set up the heap in the child after fork(), so this sgtm

If really needed then we could add a compile-time switch for this. 


sgtm. +1 to the compile time switch, so that we can disable it if there are unforeseen problems.

Sign in to add a comment