From f895b5a58ec930d3ca49ebf34959a598c7669d77 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 5 Nov 2018 04:52:50 +0800 Subject: [PATCH] src: cache the result of GetOptions() in JS land Instead of calling into C++ each time we need to check the value of a command line option, cache the option map in a new `internal/options` module for faster access to the values in JS land. PR-URL: https://github.com/nodejs/node/pull/24091 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan Reviewed-By: Refael Ackermann --- lib/crypto.js | 4 ++-- lib/internal/bash_completion.js | 3 +-- lib/internal/bootstrap/loaders.js | 3 +-- lib/internal/bootstrap/node.js | 16 ++++++++-------- lib/internal/modules/cjs/helpers.js | 4 ++-- lib/internal/modules/cjs/loader.js | 8 ++++---- lib/internal/modules/esm/default_resolve.js | 6 +++--- lib/internal/options.js | 18 ++++++++++++++++++ lib/internal/print_help.js | 5 +++-- lib/internal/process/esm_loader.js | 2 +- lib/repl.js | 2 +- lib/vm.js | 2 +- node.gyp | 1 + src/node_options.cc | 17 ++--------------- test/parallel/test-bootstrap-modules.js | 2 +- 15 files changed, 49 insertions(+), 44 deletions(-) create mode 100644 lib/internal/options.js diff --git a/lib/crypto.js b/lib/crypto.js index 2a41768414a..2645e78e5eb 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -35,8 +35,8 @@ const { ERR_CRYPTO_FIPS_UNAVAILABLE } = require('internal/errors').codes; const constants = internalBinding('constants').crypto; -const { getOptions } = internalBinding('options'); -const pendingDeprecation = getOptions('--pending-deprecation'); +const { getOptionValue } = require('internal/options'); +const pendingDeprecation = getOptionValue('--pending-deprecation'); const { fipsMode, fipsForced diff --git a/lib/internal/bash_completion.js b/lib/internal/bash_completion.js index cb8eab9ceec..13363e8c4b8 100644 --- a/lib/internal/bash_completion.js +++ b/lib/internal/bash_completion.js @@ -1,8 +1,7 @@ 'use strict'; -const { getOptions } = internalBinding('options'); +const { options, aliases } = require('internal/options'); function print(stream) { - const { options, aliases } = getOptions(); const all_opts = [...options.keys(), ...aliases.keys()]; stream.write(`_node_complete() { diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index ec67df59ce5..6e06e560a57 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -251,8 +251,7 @@ NativeModule.isInternal = function(id) { return id.startsWith('internal/') || - (id === 'worker_threads' && - !internalBinding('options').getOptions('--experimental-worker')); + (id === 'worker_threads' && !config.experimentalWorker); }; } diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index d95f1aa2939..a0cf67b9ead 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -103,12 +103,13 @@ NativeModule.require('internal/inspector_async_hook').setup(); } - const { getOptions } = internalBinding('options'); - const helpOption = getOptions('--help'); - const completionBashOption = getOptions('--completion-bash'); - const experimentalModulesOption = getOptions('--experimental-modules'); - const experimentalVMModulesOption = getOptions('--experimental-vm-modules'); - const experimentalWorkerOption = getOptions('--experimental-worker'); + const { getOptionValue } = NativeModule.require('internal/options'); + const helpOption = getOptionValue('--help'); + const completionBashOption = getOptionValue('--completion-bash'); + const experimentalModulesOption = getOptionValue('--experimental-modules'); + const experimentalVMModulesOption = + getOptionValue('--experimental-vm-modules'); + const experimentalWorkerOption = getOptionValue('--experimental-worker'); if (helpOption) { NativeModule.require('internal/print_help').print(process.stdout); return; @@ -721,10 +722,9 @@ const get = () => { const { - getOptions, envSettings: { kAllowedInEnvironment } } = internalBinding('options'); - const { options, aliases } = getOptions(); + const { options, aliases } = NativeModule.require('internal/options'); const allowedNodeEnvironmentFlags = []; for (const [name, info] of options) { diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index cda94987fad..b08e97b29c5 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -9,7 +9,7 @@ const { CHAR_HASH, } = require('internal/constants'); -const { getOptions } = internalBinding('options'); +const { getOptionValue } = require('internal/options'); // Invoke with makeRequireFunction(module) where |module| is the Module object // to use as the context for the require() function. @@ -107,7 +107,7 @@ const builtinLibs = [ 'v8', 'vm', 'zlib' ]; -if (getOptions('--experimental-worker')) { +if (getOptionValue('--experimental-worker')) { builtinLibs.push('worker_threads'); builtinLibs.sort(); } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 4a5fc596c05..506bae4b787 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -41,10 +41,10 @@ const { stripBOM, stripShebang } = require('internal/modules/cjs/helpers'); -const options = internalBinding('options'); -const preserveSymlinks = options.getOptions('--preserve-symlinks'); -const preserveSymlinksMain = options.getOptions('--preserve-symlinks-main'); -const experimentalModules = options.getOptions('--experimental-modules'); +const { getOptionValue } = require('internal/options'); +const preserveSymlinks = getOptionValue('--preserve-symlinks'); +const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); +const experimentalModules = getOptionValue('--experimental-modules'); const { ERR_INVALID_ARG_TYPE, diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 7654ca91129..9aa54d09a1b 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -6,9 +6,9 @@ const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); const { realpathSync } = require('fs'); -const { getOptions } = internalBinding('options'); -const preserveSymlinks = getOptions('--preserve-symlinks'); -const preserveSymlinksMain = getOptions('--preserve-symlinks-main'); +const { getOptionValue } = require('internal/options'); +const preserveSymlinks = getOptionValue('--preserve-symlinks'); +const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const { ERR_MISSING_MODULE, ERR_MODULE_RESOLUTION_LEGACY, diff --git a/lib/internal/options.js b/lib/internal/options.js new file mode 100644 index 00000000000..e494787b96c --- /dev/null +++ b/lib/internal/options.js @@ -0,0 +1,18 @@ +'use strict'; + +const { getOptions } = internalBinding('options'); +const { options, aliases } = getOptions(); + +function getOptionValue(option) { + const result = options.get(option); + if (!result) { + return undefined; + } + return result.value; +} + +module.exports = { + options, + aliases, + getOptionValue +}; diff --git a/lib/internal/print_help.js b/lib/internal/print_help.js index 8acc9271b19..d9ab3c1ad84 100644 --- a/lib/internal/print_help.js +++ b/lib/internal/print_help.js @@ -1,5 +1,6 @@ 'use strict'; -const { getOptions, types } = internalBinding('options'); + +const { types } = internalBinding('options'); const typeLookup = []; for (const key of Object.keys(types)) @@ -132,7 +133,7 @@ function format({ options, aliases = new Map(), firstColumn, secondColumn }) { } function print(stream) { - const { options, aliases } = getOptions(); + const { options, aliases } = require('internal/options'); // Use 75 % of the available width, and at least 70 characters. const width = Math.max(70, (stream.columns || 0) * 0.75); diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 72d39a69600..f81053a1c3c 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -48,7 +48,7 @@ exports.ESMLoader = undefined; exports.setup = function() { let ESMLoader = new Loader(); const loaderPromise = (async () => { - const userLoader = internalBinding('options').getOptions('--loader'); + const userLoader = require('internal/options').getOptionValue('--loader'); if (userLoader) { const hooks = await ESMLoader.import( userLoader, pathToFileURL(`${process.cwd()}/`).href); diff --git a/lib/repl.js b/lib/repl.js index 83ac339fdbb..3391a94396d 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -70,7 +70,7 @@ const { ERR_SCRIPT_EXECUTION_INTERRUPTED } = require('internal/errors').codes; const { sendInspectorCommand } = require('internal/util/inspector'); -const experimentalREPLAwait = internalBinding('options').getOptions( +const experimentalREPLAwait = require('internal/options').getOptionValue( '--experimental-repl-await' ); const { isRecoverableError } = require('internal/repl/recoverable'); diff --git a/lib/vm.js b/lib/vm.js index 6e735bca4a7..0cccd284bfc 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -418,7 +418,7 @@ module.exports = { compileFunction, }; -if (internalBinding('options').getOptions('--experimental-vm-modules')) { +if (require('internal/options').getOptionValue('--experimental-vm-modules')) { const { SourceTextModule } = require('internal/vm/source_text_module'); module.exports.SourceTextModule = SourceTextModule; } diff --git a/node.gyp b/node.gyp index 5a42d2b7149..1b675458d7e 100644 --- a/node.gyp +++ b/node.gyp @@ -133,6 +133,7 @@ 'lib/internal/modules/esm/translators.js', 'lib/internal/safe_globals.js', 'lib/internal/net.js', + 'lib/internal/options.js', 'lib/internal/print_help.js', 'lib/internal/priority_queue.js', 'lib/internal/process/esm_loader.js', diff --git a/src/node_options.cc b/src/node_options.cc index 4951679994a..98beb9f4c66 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -365,9 +365,8 @@ HostPort SplitHostPort(const std::string& arg, ParseAndValidatePort(arg.substr(colon + 1), errors) }; } -// Usage: Either: -// - getOptions() to get all options + metadata or -// - getOptions(string) to get the value of a particular option +// Return a map containing all the options and their metadata as well +// as the aliases void GetOptions(const FunctionCallbackInfo& args) { Mutex::ScopedLock lock(per_process_opts_mutex); Environment* env = Environment::GetCurrent(args); @@ -388,13 +387,8 @@ void GetOptions(const FunctionCallbackInfo& args) { const auto& parser = PerProcessOptionsParser::instance; - std::string filter; - if (args[0]->IsString()) filter = *node::Utf8Value(isolate, args[0]); - Local options = Map::New(isolate); for (const auto& item : parser.options_) { - if (!filter.empty() && item.first != filter) continue; - Local value; const auto& option_info = item.second; auto field = option_info.field; @@ -443,11 +437,6 @@ void GetOptions(const FunctionCallbackInfo& args) { } CHECK(!value.IsEmpty()); - if (!filter.empty()) { - args.GetReturnValue().Set(value); - return; - } - Local name = ToV8Value(context, item.first).ToLocalChecked(); Local info = Object::New(isolate); Local help_text; @@ -469,8 +458,6 @@ void GetOptions(const FunctionCallbackInfo& args) { } } - if (!filter.empty()) return; - Local aliases; if (!ToV8Value(context, parser.aliases_).ToLocal(&aliases)) return; diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 885a58ff9f8..70011637e08 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -11,4 +11,4 @@ const list = process.moduleLoadList.slice(); const assert = require('assert'); -assert(list.length <= 77, list); +assert(list.length <= 78, list);