From dc9f7e1a37d38ca52abc5cfe3c71715d2ce33c1a Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Wed, 24 Oct 2018 00:09:42 +0530 Subject: [PATCH 1/9] lib: modify NativeModule to use compileFunction Modify the NativeModule class to use compileFunction instead of manually wrapping the string source code and then compiling it. --- lib/internal/bootstrap/loaders.js | 36 +++++++++++-------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index dff6ce661a4640..e80fab3bd246fb 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -109,7 +109,6 @@ // Create this WeakMap in js-land because V8 has no C++ API for WeakMap internalBinding('module_wrap').callbackMap = new WeakMap(); - const { ContextifyScript } = internalBinding('contextify'); // Set up NativeModule function NativeModule(id) { @@ -120,7 +119,6 @@ this.exportKeys = undefined; this.loaded = false; this.loading = false; - this.script = null; // The ContextifyScript of the module } NativeModule._source = getInternalBinding('natives'); @@ -220,15 +218,6 @@ return NativeModule._source[id]; }; - NativeModule.wrap = function(script) { - return NativeModule.wrapper[0] + script + NativeModule.wrapper[1]; - }; - - NativeModule.wrapper = [ - '(function (exports, require, module, process, internalBinding) {', - '\n});' - ]; - const getOwn = (target, property, receiver) => { return ReflectApply(ObjectHasOwnProperty, target, [property]) ? ReflectGet(target, property, receiver) : @@ -237,8 +226,7 @@ NativeModule.prototype.compile = function() { const id = this.id; - let source = NativeModule.getSource(id); - source = NativeModule.wrap(source); + const source = NativeModule.getSource(id); this.loading = true; @@ -270,29 +258,29 @@ const cache = codeCacheHash[id] && (codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined; - // (code, filename, lineOffset, columnOffset - // cachedData, produceCachedData, parsingContext) - const script = new ContextifyScript( - source, this.filename, 0, 0, - cache, false, undefined + const fn = internalBinding('contextify').compileFunction( + source, + this.filename, + 0, + 0, + cache, + false, + undefined, + [], + ['exports', 'require', 'module', 'process', 'internalBinding'] ); - // This will be used to create code cache in tools/generate_code_cache.js - this.script = script; - // One of these conditions may be false when any of the inputs // of the `node_js2c` target in node.gyp is modified. // FIXME(joyeecheung): Figure out how to resolve the dependency issue. // When the code cache was introduced we were at a point where refactoring // node.gyp may not be worth the effort. - if (!cache || script.cachedDataRejected) { + if (!cache) { compiledWithoutCache.push(this.id); } else { compiledWithCache.push(this.id); } - // Arguments: timeout, displayErrors, breakOnSigint - const fn = script.runInThisContext(-1, true, false); const requireFn = this.id.startsWith('internal/deps/') ? NativeModule.requireForDeps : NativeModule.require; From efc0892947077e82a40584609a69399537f997c4 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Wed, 24 Oct 2018 02:58:38 +0530 Subject: [PATCH 2/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index e80fab3bd246fb..3a1f044eb590ef 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -109,6 +109,7 @@ // Create this WeakMap in js-land because V8 has no C++ API for WeakMap internalBinding('module_wrap').callbackMap = new WeakMap(); + const { compileFunction } = internalBinding('contextify'); // Set up NativeModule function NativeModule(id) { @@ -258,7 +259,7 @@ const cache = codeCacheHash[id] && (codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined; - const fn = internalBinding('contextify').compileFunction( + const fn = compileFunction( source, this.filename, 0, From 19262a0d4ac39dacc96af990b4642c69a49c39a9 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Wed, 24 Oct 2018 17:52:43 +0530 Subject: [PATCH 3/9] vm: report cache rejection in vm.compileFunction Add functionality in vm.compileFunction/CompileFunction to report if cache data was provided yet rejected by v8. --- src/node_contextify.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 3fbc616fda0284..4f824b7ea77868 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1099,12 +1099,23 @@ void ContextifyContext::CompileFunction( env->cached_data_string(), buf.ToLocalChecked()).IsNothing()) return; } + + // Report if cache was produced. if (fun->Set( parsing_context, env->cached_data_produced_string(), Boolean::New(isolate, cached_data_produced)).IsNothing()) return; } + // Report if cache was rejected. + if (options == ScriptCompiler::kConsumeCodeCache) { + if (fun->Set(parsing_context, + env->cached_data_rejected_string(), + Boolean::New(isolate, source.GetCachedData()->rejected)) + .IsNothing()) + return; + } + args.GetReturnValue().Set(fun); } From 9ec01a2f54291b3e9b092aca72bfff99493215c8 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Wed, 24 Oct 2018 17:53:46 +0530 Subject: [PATCH 4/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 3a1f044eb590ef..6c958b88480a53 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -276,7 +276,7 @@ // FIXME(joyeecheung): Figure out how to resolve the dependency issue. // When the code cache was introduced we were at a point where refactoring // node.gyp may not be worth the effort. - if (!cache) { + if (!cache || fn.cachedDataRejected) { compiledWithoutCache.push(this.id); } else { compiledWithCache.push(this.id); From 382e3e800246b7183fcc4f2b6e1f2aa7d04c46b7 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Thu, 25 Oct 2018 05:52:19 +0530 Subject: [PATCH 5/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 6c958b88480a53..57f82a6d58d18e 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -260,14 +260,14 @@ (codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined; const fn = compileFunction( - source, - this.filename, - 0, - 0, - cache, - false, - undefined, - [], + source, // source code + this.filename, // filename + 0, // line offset + 0, // column offset + cache, // cached data + false, // produce cached data + undefined, // parsing context + undefined, // context extensions ['exports', 'require', 'module', 'process', 'internalBinding'] ); From 72d542b55cee9e2b85634eb68bb77a42a9a19f95 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Thu, 25 Oct 2018 21:17:07 +0530 Subject: [PATCH 6/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 57f82a6d58d18e..82ee4e6c9a5435 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -268,6 +268,7 @@ false, // produce cached data undefined, // parsing context undefined, // context extensions + // arguments to the function ['exports', 'require', 'module', 'process', 'internalBinding'] ); From 690fe8b72b8135c01c06cce42fe3f6bb0624d351 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Fri, 26 Oct 2018 01:34:47 +0530 Subject: [PATCH 7/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 82ee4e6c9a5435..d37ec7fd642c0c 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -259,6 +259,8 @@ const cache = codeCacheHash[id] && (codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined; + const params = ['exports', 'require', 'module', 'process', 'internalBinding']; + const fn = compileFunction( source, // source code this.filename, // filename @@ -268,8 +270,7 @@ false, // produce cached data undefined, // parsing context undefined, // context extensions - // arguments to the function - ['exports', 'require', 'module', 'process', 'internalBinding'] + params // arguments to the function ); // One of these conditions may be false when any of the inputs From ab4d9ad0a8cf7540427cb45d314138aec51d4f9a Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sat, 27 Oct 2018 20:17:03 +0530 Subject: [PATCH 8/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index d37ec7fd642c0c..61f765f70a41c8 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -261,6 +261,7 @@ const params = ['exports', 'require', 'module', 'process', 'internalBinding']; + // TODO(@joyeecheung): figure out way to expose wrapped source for code cache const fn = compileFunction( source, // source code this.filename, // filename From d2dcd54dea3b200d577ff28c2e490b002ace5003 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Tue, 30 Oct 2018 01:50:24 +0530 Subject: [PATCH 9/9] fixup! lib: modify NativeModule to use compileFunction --- lib/internal/bootstrap/loaders.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 61f765f70a41c8..f3779cce7f4113 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -225,6 +225,14 @@ undefined; }; + NativeModule.parameters = Object.freeze([ + 'exports', + 'require', + 'module', + 'process', + 'internalBinding' + ]); + NativeModule.prototype.compile = function() { const id = this.id; const source = NativeModule.getSource(id); @@ -259,19 +267,17 @@ const cache = codeCacheHash[id] && (codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined; - const params = ['exports', 'require', 'module', 'process', 'internalBinding']; - - // TODO(@joyeecheung): figure out way to expose wrapped source for code cache + // TODO(@joyee): figure out way to expose wrapped source for code cache const fn = compileFunction( - source, // source code - this.filename, // filename - 0, // line offset - 0, // column offset - cache, // cached data - false, // produce cached data - undefined, // parsing context - undefined, // context extensions - params // arguments to the function + source, // source code + this.filename, // filename + 0, // line offset + 0, // column offset + cache, // cached data + false, // produce cached data + undefined, // parsing context + undefined, // context extensions + NativeModule.parameters // arguments to the function ); // One of these conditions may be false when any of the inputs