Skip to content

Commit 21e9aa2

Browse files
joyeecheungrvagg
authored andcommitted
src: use STL containers instead of v8 values for static module data
Instead of putting the source code and the cache in v8::Objects, put them in per-process std::maps. This has the following benefits: - It's slightly lighter in weight compared to storing things on the v8 heap. Also it may be slightly faster since the preivous v8::Object is already in dictionary mode - though the difference is very small given the number of native modules is limited. - The source and code cache generation templates are now much simpler since they just initialize static arrays and manipulate STL constructs. - The static native module data can be accessed independently of any Environment or Isolate, and it's easy to look them up from the C++'s side. - It's now impossible to mutate the source code used to compile native modules from the JS land since it's completely separate from the v8 heap. We can still get the constant strings from process.binding('natives') but that's all. A few drive-by fixes: - Remove DecorateErrorStack in LookupAndCompile - We don't need to capture the exception to decorate when we encounter errors during native module compilation, as those errors should be syntax errors and v8 is able to decorate them well. We use CompileFunctionInContext so there is no need to worry about wrappers either. - The code cache could be rejected when node is started with v8 flags. Instead of aborting in that case, simply keep a record in the native_module_without_cache set. - Refactor js2c.py a bit, reduce code duplication and inline Render() to make the one-byte/two-byte special treatment easier to read. PR-URL: #24384 Fixes: https://github.com/Remove Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0a08cd7 commit 21e9aa2

16 files changed

+457
-419
lines changed

lib/internal/bootstrap/cache.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ const {
99
NativeModule
1010
} = require('internal/bootstrap/loaders');
1111
const {
12-
source,
12+
getSource,
1313
compileCodeCache
1414
} = internalBinding('native_module');
1515
const { hasTracing } = process.binding('config');
1616

17+
const source = getSource();
1718
const depsModule = Object.keys(source).filter(
1819
(key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps')
1920
);

node.gyp

+1-2
Original file line numberDiff line numberDiff line change
@@ -410,15 +410,13 @@
410410
'src/node_api.h',
411411
'src/node_api_types.h',
412412
'src/node_buffer.h',
413-
'src/node_code_cache.h',
414413
'src/node_constants.h',
415414
'src/node_contextify.h',
416415
'src/node_errors.h',
417416
'src/node_file.h',
418417
'src/node_http2.h',
419418
'src/node_http2_state.h',
420419
'src/node_internals.h',
421-
'src/node_javascript.h',
422420
'src/node_messaging.h',
423421
'src/node_mutex.h',
424422
'src/node_native_module.h',
@@ -430,6 +428,7 @@
430428
'src/node_persistent.h',
431429
'src/node_platform.h',
432430
'src/node_root_certs.h',
431+
'src/node_union_bytes.h',
433432
'src/node_version.h',
434433
'src/node_watchdog.h',
435434
'src/node_revert.h',

src/env.cc

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
#include "node_internals.h"
21
#include "async_wrap.h"
3-
#include "v8-profiler.h"
42
#include "node_buffer.h"
5-
#include "node_platform.h"
6-
#include "node_file.h"
73
#include "node_context_data.h"
4+
#include "node_file.h"
5+
#include "node_internals.h"
6+
#include "node_native_module.h"
7+
#include "node_platform.h"
88
#include "node_worker.h"
99
#include "tracing/agent.h"
1010
#include "tracing/traced_value.h"
11+
#include "v8-profiler.h"
1112

1213
#include <stdio.h>
1314
#include <algorithm>

src/env.h

+6-9
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@
2929
#include "inspector_agent.h"
3030
#endif
3131
#include "handle_wrap.h"
32+
#include "node.h"
33+
#include "node_http2_state.h"
34+
#include "node_options.h"
3235
#include "req_wrap.h"
3336
#include "util.h"
3437
#include "uv.h"
3538
#include "v8.h"
36-
#include "node.h"
37-
#include "node_options.h"
38-
#include "node_http2_state.h"
3939

4040
#include <list>
4141
#include <stdint.h>
@@ -347,12 +347,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
347347
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
348348
V(message_port, v8::Object) \
349349
V(message_port_constructor_template, v8::FunctionTemplate) \
350-
V(native_modules_code_cache, v8::Object) \
351-
V(native_modules_code_cache_hash, v8::Object) \
352-
V(native_modules_source, v8::Object) \
353-
V(native_modules_source_hash, v8::Object) \
354-
V(native_modules_with_cache, v8::Set) \
355-
V(native_modules_without_cache, v8::Set) \
356350
V(performance_entry_callback, v8::Function) \
357351
V(performance_entry_template, v8::Function) \
358352
V(pipe_constructor_template, v8::FunctionTemplate) \
@@ -684,6 +678,9 @@ class Environment {
684678
// List of id's that have been destroyed and need the destroy() cb called.
685679
inline std::vector<double>* destroy_async_id_list();
686680

681+
std::set<std::string> native_modules_with_cache;
682+
std::set<std::string> native_modules_without_cache;
683+
687684
std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
688685
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
689686
std::unordered_map<uint32_t, contextify::ContextifyScript*>

src/node.cc

+19-17
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include "node_context_data.h"
2525
#include "node_errors.h"
2626
#include "node_internals.h"
27-
#include "node_javascript.h"
2827
#include "node_native_module.h"
2928
#include "node_perf.h"
3029
#include "node_platform.h"
@@ -130,7 +129,7 @@ typedef int mode_t;
130129

131130
namespace node {
132131

133-
using native_module::NativeModule;
132+
using native_module::NativeModuleLoader;
134133
using options_parser::kAllowedInEnvironment;
135134
using options_parser::kDisallowedInEnvironment;
136135
using v8::Array;
@@ -212,7 +211,7 @@ double prog_start_time;
212211
Mutex per_process_opts_mutex;
213212
std::shared_ptr<PerProcessOptions> per_process_opts {
214213
new PerProcessOptions() };
215-
214+
NativeModuleLoader per_process_loader;
216215
static Mutex node_isolate_mutex;
217216
static Isolate* node_isolate;
218217

@@ -1243,8 +1242,7 @@ static void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
12431242
Null(env->isolate())).FromJust());
12441243
DefineConstants(env->isolate(), exports);
12451244
} else if (!strcmp(*module_v, "natives")) {
1246-
exports = Object::New(env->isolate());
1247-
NativeModule::GetNatives(env, exports);
1245+
exports = per_process_loader.GetSourceObject(env->context());
12481246
} else {
12491247
return ThrowIfNoSuchModule(env, *module_v);
12501248
}
@@ -1780,18 +1778,24 @@ void LoadEnvironment(Environment* env) {
17801778

17811779
// The bootstrapper scripts are lib/internal/bootstrap/loaders.js and
17821780
// lib/internal/bootstrap/node.js, each included as a static C string
1783-
// defined in node_javascript.h, generated in node_javascript.cc by
1784-
// node_js2c.
1781+
// generated in node_javascript.cc by node_js2c.
17851782

1786-
// TODO(joyeecheung): use NativeModule::Compile
1783+
// TODO(joyeecheung): use NativeModuleLoader::Compile
1784+
// We duplicate the string literals here since once we refactor the bootstrap
1785+
// compilation out to NativeModuleLoader none of this is going to matter
1786+
Isolate* isolate = env->isolate();
17871787
Local<String> loaders_name =
1788-
FIXED_ONE_BYTE_STRING(env->isolate(), "internal/bootstrap/loaders.js");
1788+
FIXED_ONE_BYTE_STRING(isolate, "internal/bootstrap/loaders.js");
1789+
Local<String> loaders_source =
1790+
per_process_loader.GetSource(isolate, "internal/bootstrap/loaders");
17891791
MaybeLocal<Function> loaders_bootstrapper =
1790-
GetBootstrapper(env, LoadersBootstrapperSource(env), loaders_name);
1792+
GetBootstrapper(env, loaders_source, loaders_name);
17911793
Local<String> node_name =
1792-
FIXED_ONE_BYTE_STRING(env->isolate(), "internal/bootstrap/node.js");
1794+
FIXED_ONE_BYTE_STRING(isolate, "internal/bootstrap/node.js");
1795+
Local<String> node_source =
1796+
per_process_loader.GetSource(isolate, "internal/bootstrap/node");
17931797
MaybeLocal<Function> node_bootstrapper =
1794-
GetBootstrapper(env, NodeBootstrapperSource(env), node_name);
1798+
GetBootstrapper(env, node_source, node_name);
17951799

17961800
if (loaders_bootstrapper.IsEmpty() || node_bootstrapper.IsEmpty()) {
17971801
// Execution was interrupted.
@@ -1843,8 +1847,6 @@ void LoadEnvironment(Environment* env) {
18431847
env->options()->debug_options->break_node_first_line)
18441848
};
18451849

1846-
NativeModule::LoadBindings(env);
1847-
18481850
// Bootstrap internal loaders
18491851
Local<Value> bootstrapped_loaders;
18501852
if (!ExecuteBootstrapper(env, loaders_bootstrapper.ToLocalChecked(),
@@ -2485,7 +2487,6 @@ void FreePlatform(MultiIsolatePlatform* platform) {
24852487
delete platform;
24862488
}
24872489

2488-
24892490
Local<Context> NewContext(Isolate* isolate,
24902491
Local<ObjectTemplate> object_template) {
24912492
auto context = Context::New(isolate, nullptr, object_template);
@@ -2499,8 +2500,9 @@ Local<Context> NewContext(Isolate* isolate,
24992500
// Run lib/internal/per_context.js
25002501
Context::Scope context_scope(context);
25012502

2502-
// TODO(joyeecheung): use NativeModule::Compile
2503-
Local<String> per_context = NodePerContextSource(isolate);
2503+
// TODO(joyeecheung): use NativeModuleLoader::Compile
2504+
Local<String> per_context =
2505+
per_process_loader.GetSource(isolate, "internal/per_context");
25042506
ScriptCompiler::Source per_context_src(per_context, nullptr);
25052507
Local<Script> s = ScriptCompiler::Compile(
25062508
context,

src/node_code_cache.h

-19
This file was deleted.

src/node_code_cache_stub.cc

+9-13
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11

2-
#include "node_code_cache.h"
2+
#include "node_native_module.h"
33

44
// This is supposed to be generated by tools/generate_code_cache.js
55
// The stub here is used when configure is run without `--code-cache-path`
66

77
namespace node {
8+
namespace native_module {
89

9-
const bool native_module_has_code_cache = false;
10+
// The generated source code would insert <std::string, UnionString> pairs
11+
// into native_module_loader.code_cache_.
12+
void NativeModuleLoader::LoadCodeCache() {}
1013

11-
void DefineCodeCache(Environment* env, v8::Local<v8::Object> target) {
12-
// When we do not produce code cache for builtin modules,
13-
// `internalBinding('code_cache')` returns an empty object
14-
// (here as `target`) so this is a noop.
15-
}
16-
17-
void DefineCodeCacheHash(Environment* env, v8::Local<v8::Object> target) {
18-
// When we do not produce code cache for builtin modules,
19-
// `internalBinding('code_cache_hash')` returns an empty object
20-
// (here as `target`) so this is a noop.
21-
}
14+
// The generated source code would instert <std::string, std::string> pairs
15+
// into native_module_loader.code_cache_hash_.
16+
void NativeModuleLoader::LoadCodeCacheHash() {}
2217

18+
} // namespace native_module
2319
} // namespace node

src/node_internals.h

+5
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ struct sockaddr;
171171

172172
namespace node {
173173

174+
namespace native_module {
175+
class NativeModuleLoader;
176+
}
177+
174178
extern Mutex process_mutex;
175179
extern Mutex environ_mutex;
176180

@@ -179,6 +183,7 @@ extern bool v8_initialized;
179183

180184
extern Mutex per_process_opts_mutex;
181185
extern std::shared_ptr<PerProcessOptions> per_process_opts;
186+
extern native_module::NativeModuleLoader per_process_loader;
182187

183188
// Forward declaration
184189
class Environment;

src/node_javascript.h

-41
This file was deleted.

0 commit comments

Comments
 (0)