Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6291c10

Browse files
joyeecheungrichardlau
authored andcommitted
vm: use default HDO when importModuleDynamically is not set
This makes it possile to hit the in-isolate compilation cache whenhost-defined options are not necessary.PR-URL:#49950Backport-PR-URL:#51004Refs:#35375Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>Reviewed-By: Chengzhong Wu <legendecas@gmail.com>Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent5e42651 commit6291c10

File tree

6 files changed

+81
-6
lines changed

6 files changed

+81
-6
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
// This benchmarks compiling scripts that hit the in-isolate compilation
4+
// cache (by having the same source).
5+
constcommon=require('../common.js');
6+
constfs=require('fs');
7+
constvm=require('vm');
8+
constfixtures=require('../../test/common/fixtures.js');
9+
constscriptPath=fixtures.path('snapshot','typescript.js');
10+
11+
constbench=common.createBenchmark(main,{
12+
type:['with-dynamic-import-callback','without-dynamic-import-callback'],
13+
n:[100],
14+
});
15+
16+
constscriptSource=fs.readFileSync(scriptPath,'utf8');
17+
18+
functionmain({ n, type}){
19+
letscript;
20+
bench.start();
21+
constoptions={};
22+
switch(type){
23+
case'with-dynamic-import-callback':
24+
// Use a dummy callback for now until we really need to benchmark it.
25+
options.importModuleDynamically=async()=>{};
26+
break;
27+
case'without-dynamic-import-callback':
28+
break;
29+
}
30+
for(leti=0;i<n;i++){
31+
script=newvm.Script(scriptSource,options);
32+
}
33+
bench.end(n);
34+
script.runInThisContext();
35+
}

‎lib/internal/modules/esm/utils.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ const {
1212
host_defined_option_symbol,
1313
},
1414
}=internalBinding('util');
15+
const{
16+
default_host_defined_options,
17+
}=internalBinding('symbols');
18+
1519
const{
1620
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
1721
ERR_INVALID_ARG_VALUE,
@@ -128,6 +132,13 @@ const moduleRegistries = new SafeWeakMap();
128132
*/
129133
functionregisterModule(referrer,registry){
130134
constidSymbol=referrer[host_defined_option_symbol];
135+
if(idSymbol===default_host_defined_options){
136+
// The referrer is compiled without custom callbacks, so there is
137+
// no registry to hold on to. We'll throw
138+
// ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is
139+
// needed.
140+
return;
141+
}
131142
// To prevent it from being GC'ed.
132143
registry.callbackReferrer??=referrer;
133144
moduleRegistries.set(idSymbol,registry);

‎lib/vm.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ class Script extends ContextifyScript {
8787
}
8888
validateBoolean(produceCachedData,'options.produceCachedData');
8989

90+
if(importModuleDynamically!==undefined){
91+
// Check that it's either undefined or a function before we pass
92+
// it into the native constructor.
93+
validateFunction(importModuleDynamically,
94+
'options.importModuleDynamically');
95+
}
9096
// Calling `ReThrow()` on a native TryCatch does not generate a new
9197
// abort-on-uncaught-exception check. A dummy try/catch in JS land
9298
// protects against that.
@@ -97,14 +103,13 @@ class Script extends ContextifyScript {
97103
columnOffset,
98104
cachedData,
99105
produceCachedData,
100-
parsingContext);
106+
parsingContext,
107+
importModuleDynamically!==undefined);
101108
}catch(e){
102109
throwe;/* node-do-not-add-exception-line */
103110
}
104111

105112
if(importModuleDynamically!==undefined){
106-
validateFunction(importModuleDynamically,
107-
'options.importModuleDynamically');
108113
const{ importModuleDynamicallyWrap}=require('internal/vm/module');
109114
const{ registerModule}=require('internal/modules/esm/utils');
110115
registerModule(this,{

‎src/env_properties.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
// Symbols are per-isolate primitives but Environment proxies them
3232
// for the sake of convenience.
3333
#definePER_ISOLATE_SYMBOL_PROPERTIES(V) \
34+
V(default_host_defined_options, "default_host_defined_options") \
3435
V(fs_use_promises_symbol, "fs_use_promises_symbol") \
3536
V(async_id_symbol, "async_id_symbol") \
3637
V(handle_onclose_symbol, "handle_onclose") \

‎src/node_contextify.cc

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -787,10 +787,12 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
787787
bool produce_cached_data =false;
788788
Local<Context> parsing_context = context;
789789

790+
bool needs_custom_host_defined_options =false;
790791
if (argc >2) {
791792
// new ContextifyScript(code, filename, lineOffset, columnOffset,
792-
// cachedData, produceCachedData, parsingContext)
793-
CHECK_EQ(argc,7);
793+
// cachedData, produceCachedData, parsingContext,
794+
// needsCustomHostDefinedOptions)
795+
CHECK_EQ(argc,8);
794796
CHECK(args[2]->IsNumber());
795797
line_offset = args[2].As<Int32>()->Value();
796798
CHECK(args[3]->IsNumber());
@@ -809,6 +811,9 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
809811
CHECK_NOT_NULL(sandbox);
810812
parsing_context = sandbox->context();
811813
}
814+
if (args[7]->IsTrue()) {
815+
needs_custom_host_defined_options =true;
816+
}
812817
}
813818

814819
ContextifyScript* contextify_script =
@@ -832,7 +837,12 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
832837

833838
Local<PrimitiveArray> host_defined_options =
834839
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
835-
Local<Symbol> id_symbol =Symbol::New(isolate, filename);
840+
// We need a default host defined options that's the same for all scripts
841+
// not needing custom module callbacks for so that the isolate compilation
842+
// cache can be hit.
843+
Local<Symbol> id_symbol = needs_custom_host_defined_options
844+
?Symbol::New(isolate, filename)
845+
: env->default_host_defined_options();
836846
host_defined_options->Set(
837847
isolate, loader::HostDefinedOptions::kID, id_symbol);
838848

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
3+
require('../common');
4+
const{ Script}=require('vm');
5+
constassert=require('assert');
6+
7+
assert.rejects(async()=>{
8+
constscript=newScript('import("fs")');
9+
constimported=script.runInThisContext();
10+
awaitimported;
11+
},{
12+
code:'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING'
13+
});

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp