From 58afb76286305e8a881558dbc974da1984fa145e Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 13 Feb 2020 15:24:55 -0800 Subject: [PATCH 1/6] Revert "src: unregister Isolate with platform before disposing" This reverts commit 65e5a8a90cd85fc4d9036bc4b54e0905edd8b51a. --- src/node.h | 1 - src/node_main_instance.cc | 2 +- src/node_worker.cc | 7 +------ test/cctest/node_test_fixture.h | 2 +- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/node.h b/src/node.h index aadb60ea557f43..d6ec670ae73b7a 100644 --- a/src/node.h +++ b/src/node.h @@ -280,7 +280,6 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform { // This function may only be called once per `Isolate`, and discard any // pending delayed tasks scheduled for that isolate. - // This needs to be called right before calling `Isolate::Dispose()`. virtual void UnregisterIsolate(v8::Isolate* isolate) = 0; // The platform should call the passed function once all state associated diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 86a857299f6e90..eea847725300f0 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -99,8 +99,8 @@ NodeMainInstance::~NodeMainInstance() { if (!owns_isolate_) { return; } - platform_->UnregisterIsolate(isolate_); isolate_->Dispose(); + platform_->UnregisterIsolate(isolate_); } int NodeMainInstance::Run() { diff --git a/src/node_worker.cc b/src/node_worker.cc index b89166e9f48ad5..f1b2347d29cbe3 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -192,13 +192,8 @@ class WorkerThreadData { *static_cast(data) = true; }, &platform_finished); - // The order of these calls is important; if the Isolate is first disposed - // and then unregistered, there is a race condition window in which no - // new Isolate at the same address can successfully be registered with - // the platform. - // (Refs: https://github.com/nodejs/node/issues/30846) - w_->platform_->UnregisterIsolate(isolate); isolate->Dispose(); + w_->platform_->UnregisterIsolate(isolate); // Wait until the platform has cleaned up all relevant resources. while (!platform_finished) diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index cd669d23da3384..f6b80c860c1f58 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -106,9 +106,9 @@ class NodeTestFixture : public ::testing::Test { void TearDown() override { isolate_->Exit(); + isolate_->Dispose(); platform->DrainTasks(isolate_); platform->UnregisterIsolate(isolate_); - isolate_->Dispose(); isolate_ = nullptr; } }; From faee9ac6de25e41d4c067a8dc2f379c9d1b70e09 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 13 Feb 2020 15:26:33 -0800 Subject: [PATCH 2/6] Revert "build: warn upon --use-largepages config option" This reverts commit f5cd6d73fdf8e56954214fd57933df37b5168c54. --- configure.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/configure.py b/configure.py index 6537a62d3954d8..6a01fa46378c3b 100755 --- a/configure.py +++ b/configure.py @@ -1060,13 +1060,6 @@ def configure_node(o): else: o['variables']['node_use_dtrace'] = 'false' - if options.node_use_large_pages or options.node_use_large_pages_script_lld: - warn('''The `--use-largepages` and `--use-largepages-script-lld` options - have no effect during build time. Support for mapping to large pages is - now a runtime option of Node.js. Run `node --use-largepages` or add - `--use-largepages` to the `NODE_OPTIONS` environment variable once - Node.js is built to enable mapping to large pages.''') - if options.no_ifaddrs: o['defines'] += ['SUNOS_NO_IFADDRS'] From c0360b60b109e7fb17c6c1daacf22849988ed90a Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 13 Feb 2020 15:26:56 -0800 Subject: [PATCH 3/6] Revert "src: make large_pages node.cc include conditional" This reverts commit 5e232f5de92c2340536a5065782cad57e8fe64fb. --- src/node.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index cadbd9a62db9e0..05c87c62d6db6c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -64,9 +64,7 @@ #include "inspector/worker_inspector.h" // ParentInspectorHandle #endif -#ifdef NODE_ENABLE_LARGE_CODE_PAGES #include "large_pages/node_large_page.h" -#endif #ifdef NODE_REPORT #include "node_report.h" From 125cc5b5e1646da7760e095647926cd81c1e3c67 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 13 Feb 2020 15:27:04 -0800 Subject: [PATCH 4/6] Revert "build: switch realpath to pwd" This reverts commit dea40f8c522d60b96665a1681bd4f6863a6e9f19. --- node.gypi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node.gypi b/node.gypi index 4ee7bad0104040..687da8b501ceff 100644 --- a/node.gypi +++ b/node.gypi @@ -312,7 +312,7 @@ 'llvm_version=="0.0"', { 'ldflags': [ '-Wl,-T', - ' Date: Thu, 13 Feb 2020 15:27:13 -0800 Subject: [PATCH 5/6] Revert "build: re-introduce --use-largepages as no-op" This reverts commit 5fcf542faa08e6f59428d60dbd385595597bcda8. --- configure.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/configure.py b/configure.py index 6a01fa46378c3b..210bd8ec279909 100755 --- a/configure.py +++ b/configure.py @@ -398,16 +398,6 @@ dest='with_etw', help='build with ETW (default is true on Windows)') -parser.add_option('--use-largepages', - action='store_true', - dest='node_use_large_pages', - help='This option has no effect. --use-largepages is now a runtime option.') - -parser.add_option('--use-largepages-script-lld', - action='store_true', - dest='node_use_large_pages_script_lld', - help='This option has no effect. --use-largepages is now a runtime option.') - intl_optgroup.add_option('--with-intl', action='store', dest='with_intl', From b66259fdf0da3d87c2a33abad48a318e4360e9d8 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 13 Feb 2020 15:27:47 -0800 Subject: [PATCH 6/6] Revert "src: make --use-largepages a runtime option" This reverts commit fcbd2d245ac917785cb59ff51e0f599a7886df6b. --- configure.py | 33 +++++++++++++++++++++++ doc/api/cli.md | 17 ------------ doc/node.1 | 10 ------- node.gyp | 5 ++-- node.gypi | 6 +++-- src/large_pages/node_large_page.cc | 19 ++++++------- src/node.cc | 30 +++++++-------------- src/node_options.cc | 9 ------- src/node_options.h | 1 - test/parallel/test-startup-large-pages.js | 29 -------------------- 10 files changed, 58 insertions(+), 101 deletions(-) delete mode 100644 test/parallel/test-startup-large-pages.js diff --git a/configure.py b/configure.py index 210bd8ec279909..14f5665e0299a2 100755 --- a/configure.py +++ b/configure.py @@ -398,6 +398,17 @@ dest='with_etw', help='build with ETW (default is true on Windows)') +parser.add_option('--use-largepages', + action='store_true', + dest='node_use_large_pages', + help='build with Large Pages support. This feature is supported only on Linux kernel' + + '>= 2.6.38 with Transparent Huge pages enabled and FreeBSD') + +parser.add_option('--use-largepages-script-lld', + action='store_true', + dest='node_use_large_pages_script_lld', + help='link against the LLVM ld linker script. Implies -fuse-ld=lld in the linker flags') + intl_optgroup.add_option('--with-intl', action='store', dest='with_intl', @@ -1050,6 +1061,28 @@ def configure_node(o): else: o['variables']['node_use_dtrace'] = 'false' + if options.node_use_large_pages and not flavor in ('linux', 'freebsd', 'mac'): + raise Exception( + 'Large pages are supported only on Linux, FreeBSD and MacOS Systems.') + if options.node_use_large_pages and flavor in ('linux', 'freebsd', 'mac'): + if options.shared or options.enable_static: + raise Exception( + 'Large pages are supported only while creating node executable.') + if target_arch!="x64": + raise Exception( + 'Large pages are supported only x64 platform.') + if flavor == 'mac': + info('macOS server with 32GB or more is recommended') + if flavor == 'linux': + # Example full version string: 2.6.32-696.28.1.el6.x86_64 + FULL_KERNEL_VERSION=os.uname()[2] + KERNEL_VERSION=FULL_KERNEL_VERSION.split('-')[0] + if KERNEL_VERSION < "2.6.38" and flavor == 'linux': + raise Exception( + 'Large pages need Linux kernel version >= 2.6.38') + o['variables']['node_use_large_pages'] = b(options.node_use_large_pages) + o['variables']['node_use_large_pages_script_lld'] = b(options.node_use_large_pages_script_lld) + if options.no_ifaddrs: o['defines'] += ['SUNOS_NO_IFADDRS'] diff --git a/doc/api/cli.md b/doc/api/cli.md index 979c1214f5c59e..7703d0b6bd99d2 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -881,22 +881,6 @@ environment variables. See `SSL_CERT_DIR` and `SSL_CERT_FILE`. -### `--use-largepages=mode` - - -Re-map the Node.js static code to large memory pages at startup. If supported on -the target system, this will cause the Node.js static code to be moved onto 2 -MiB pages instead of 4 KiB pages. - -The following values are valid for `mode`: -* `off`: No mapping will be attempted. This is the default. -* `on`: If supported by the OS, mapping will be attempted. Failure to map will - be ignored and a message will be printed to standard error. -* `silent`: If supported by the OS, mapping will be attempted. Failure to map - will be ignored and will not be reported. - ### `--v8-options`