This patch:
1. Refactor the routines used to compile and run an embedder
entrypoint. In JS land special handling for SEA is done
directly in main/embedding.js instead of clobbering the CJS
loader. Add warnings to remind users that currently the
require() in SEA bundled scripts only supports loading builtins.
2. Don't use the bundled SEA code cache when compiling CJS
loaded from disk, since in that case we are certainly not
compiling the code bundled into the SEA. Use a is_sea_main
flag in CompileFunctionForCJSLoader() (which replaces an unused
argument) to pass this into the C++ land - the code cache is
still read directly from C++ to avoid the overhead of
ArrayBuffer creation.
3. Move SEA loading code into
MaybeLoadSingleExecutableApplication() which calls
LoadEnvironment() with its own StartExecutionCallback().
This avoids more hidden switches in StartExecution() and
make them explicit. Also add some TODOs about how to support
ESM in embedded applications.
4. Add more comments
PR-URL: https://github.com/nodejs/node/pull/53573
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch:
1. Adds ESM syntax detection to compileFunctionForCJSLoader()
for --experimental-detect-module and allow it to emit the
warning for how to load ESM when it's used to parse ESM as
CJS but detection is not enabled.
2. Moves the ESM detection of --experimental-detect-module for
the entrypoint from executeUserEntryPoint() into
Module.prototype._compile() and handle it directly in the
CJS loader so that the errors thrown during compilation *and
execution* during the loading of the entrypoint does not
need to be bubbled all the way up. If the entrypoint doesn't
parse as CJS, and detection is enabled, the CJS loader will
re-load the entrypoint as ESM on the spot asynchronously using
runEntryPointWithESMLoader() and cascadedLoader.import(). This
is fine for the entrypoint because unlike require(ESM) we don't
the namespace of the entrypoint synchronously, and can just
ignore the returned value. In this case process.mainModule is
reset to undefined as they are not available for ESM entrypoints.
3. Supports --experimental-detect-module for require(esm).
PR-URL: https://github.com/nodejs/node/pull/52047
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Instead of using an async function wrapper, just try compiling code with
unknown module format as SourceTextModule when it cannot be compiled
as CJS and the error message indicates that it's worth a retry. If
it can be parsed as SourceTextModule then it's considered ESM.
Also, move shouldRetryAsESM() to C++ completely so that
we can reuse it in the CJS module loader for require(esm).
Drive-by: move methods that don't belong to ContextifyContext
out as static methods and move GetHostDefinedOptions to
ModuleWrap.
PR-URL: https://github.com/nodejs/node/pull/52413
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
This patch implements automatic on-disk code caching that can be enabled
via an environment variable NODE_COMPILE_CACHE.
When set, whenever Node.js compiles a CommonJS or a ECMAScript Module,
it will use on-disk [V8 code cache][] persisted in the specified
directory to speed up the compilation. This may slow down the first
load of a module graph, but subsequent loads of the same module graph
may get a significant speedup if the contents of the modules do not
change. Locally, this speeds up loading of
test/fixtures/snapshot/typescript.js from ~130ms to ~80ms.
To clean up the generated code cache, simply remove the directory.
It will be recreated the next time the same directory is used for
`NODE_COMPILE_CACHE`.
Compilation cache generated by one version of Node.js may not be used
by a different version of Node.js. Cache generated by different versions
of Node.js will be stored separately if the same directory is used
to persist the cache, so they can co-exist.
Caveat: currently when using this with V8 JavaScript code coverage, the
coverage being collected by V8 may be less precise in functions that are
deserialized from the code cache. It's recommended to turn this off when
running tests to generate precise coverage.
Implementation details:
There is one cache file per module on disk. The directory layout
is:
- Compile cache directory (from NODE_COMPILE_CACHE)
- 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION
- 2ea3424d:
- 10860e5a: CRC32 hash of filename + module type
- 431e9adc: ...
- ...
Inside the cache file, there is a header followed by the actual
cache content:
```
[uint32_t] code size
[uint32_t] code hash
[uint32_t] cache size
[uint32_t] cache hash
... compile cache content ...
```
When reading the cache file, we'll also check if the code size
and code hash match the code that the module loader is loading
and whether the cache size and cache hash match the file content
read. If they don't match, or if V8 rejects the cache passed,
we'll ignore the mismatch cache, and regenerate the cache after
compilation succeeds and rewrite it to disk.
PR-URL: https://github.com/nodejs/node/pull/52535
Refs: https://github.com/nodejs/node/issues/47472
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Previously when managing the importModuleDynamically callback of
vm.compileFunction(), we use an ID number as the host defined option
and maintain a per-Environment ID -> CompiledFnEntry map to retain
the top-level referrer function returned by vm.compileFunction() in
order to pass it back to the callback, but it would leak because with
how we used v8::Persistent to maintain this reference, V8 would not
be able to understand the cycle and would just think that the
CompiledFnEntry was supposed to live forever. We made an attempt
to make that reference known to V8 by making the CompiledFnEntry weak
and using a private symbol to make CompiledFnEntry strongly
references the top-level referrer function in
https://github.com/nodejs/node/pull/46785, but that turned out to be
unsound, because the there's no guarantee that the top-level function
must be alive while import() can still be initiated from that
function, since V8 could discard the top-level function and only keep
inner functions alive, so relying on the top-level function to keep
the CompiledFnEntry alive could result in use-after-free which caused
a revert of that fix.
With this patch we use a symbol in the host defined options instead of
a number, because with the stage-3 symbol-as-weakmap-keys proposal
we could directly use that symbol to keep the referrer alive using a
WeakMap. As a bonus this also keeps the other kinds of referrers
alive as long as import() can still be initiated from that
Script/Module, so this also fixes the long-standing crash caused by
vm.Script being GC'ed too early when its importModuleDynamically
callback still needs it.
PR-URL: https://github.com/nodejs/node/pull/48510
Refs: https://github.com/nodejs/node/issues/44211
Refs: https://github.com/nodejs/node/issues/42080
Refs: https://github.com/nodejs/node/issues/47096
Refs: https://github.com/nodejs/node/issues/43205
Refs: https://github.com/nodejs/node/issues/38695
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Previously the ContextifyContext holds a MicrotaskQueueWrap which in
turns holds a MicrotaskQueue in a shared pointer. The indirection is
actually unnecessary, we can directly hold the MicrotaskQueue via
a unique pointer in ContextifyContext, the lifetime would still
remain the same but the graph would be simpler, and this removes
the additional JS -> C++ to create the wrapper object.
PR-URL: https://github.com/nodejs/node/pull/48982
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
The binding data must be weak so that it won't keep the realm reachable
from strong GC roots indefinitely. The wrapper object of binding data
should be referenced from JavaScript, thus the binding data should be
reachable throughout the lifetime of the realm.
PR-URL: https://github.com/nodejs/node/pull/47688
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This PR is a follow-up of #46615.
When bumping V8 for node 18, various bugs appeared around vm.
Some have been fixed along the flow, but there is at least one
remaining and described at:
https://github.com/facebook/jest/issues/13338.
The current fix does nothing on node 20: the new bump of v8 done for
node 20 seems to fix it. But it would fix the problem in both node 18
and node 19. I confirmed it would fix node 19 by cherry-picking the
commit on v19.x-staging and launching `make -j4 jstest` on it.
PR-URL: https://github.com/nodejs/node/pull/47572
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.
Previously the initialization was similar to:
```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```
Now it's similar to:
```
module.exports = { property: ... };
```
PR-URL: https://github.com/nodejs/node/pull/47913
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch moves the initialization of per-isolate properties of
the bindings that are in the embedded snapshot separate from the
initialization of their per-context properties. This is necessary
for workers to share the isolate snapshot with the main thread
and deserialize these properties instead of creating them from
scratch.
PR-URL: https://github.com/nodejs/node/pull/47768
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Previously in the implementation there was a cycle that V8 could not
detect:
Strong global reference to CompiledFnEntry (JS wrapper)
-> strong reference to callback setting (through the
callbackMap key-value pair)
-> importModuleDynamically (wrapper in internalCompileFunction())
-> Strong reference to the compiled function (through closure in
internalCompileFunction())
The CompiledFnEntry only gets GC'ed when the compiled function is GC'ed.
Since the compiled function is always reachable as described above,
there is a leak.
We only needed the first strong global reference because we didn't want
the function to outlive the CompiledFnEntry. In this case it can be
solved by using a private symbol instead of going with the global
reference + destruction in the weak callback, which V8's GC is not
going to understand.
PR-URL: https://github.com/nodejs/node/pull/46785
Fixes: https://github.com/nodejs/node/issues/42080
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
A regression has been introduced in node 18.2.0,
it makes the following snippet fails while it used to work in the past:
```
const assert = require('assert');
const vm = require('vm');
const global = vm.runInContext('this', vm.createContext());
const totoSymbol = Symbol.for('toto');
Object.defineProperty(global, totoSymbol, {
enumerable: true,
writable: true,
value: 4,
configurable: true,
});
assert(Object.getOwnPropertySymbols(global).includes(totoSymbol));
```
Regression introduced by: https://github.com/nodejs/node/pull/42963.
So I basically attempted to start understanding what it changed to make
it fix the initial issue while not breaking the symbol related one.
Fixes: https://github.com/nodejs/node/issues/45983
PR-URL: https://github.com/nodejs/node/pull/46458
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Add experimental support for loading snapshots in the embedder API
by adding a public opaque wrapper for our `SnapshotData` struct and
allowing embedders to pass it to the relevant setup functions.
Where applicable, use these helpers to deduplicate existing code
in Node.js’s startup path.
This has shown a 40 % startup performance increase for a real-world
application, even with the somewhat limited current support for
built-in modules.
The documentation includes a note about no guarantees for API or
ABI stability for this feature while it is experimental.
PR-URL: https://github.com/nodejs/node/pull/45888
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Having this information available is useful for functions just as
it is for scripts. Therefore, expose it in the same way that other
information related to code caching is reported.
As part of this, de-duplify the code for setting the properties on
the C++ side and add proper exception handling to it.
PR-URL: https://github.com/nodejs/node/pull/46320
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
V8 already parses the source map magic comments. Currently, only scripts
and functions expose the parsed source map URLs. It is unnecessary to
parse the source map magic comments again when the parsed information is
available.
PR-URL: https://github.com/nodejs/node/pull/44798
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Instead of adding a reference to the ContextifyContext by using
a v8::External, we make ContextifyContext a weak BaseObject that
whose wrapper is referenced by the sandbox via a private symbol.
This makes it easier to snapshot the contexts, in addition to
reusing the BaseObject lifetime management for ContextifyContexts.
PR-URL: https://github.com/nodejs/node/pull/44796
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Access to the global object from within a vm context is intercepted
so it's slow, therefore we should try to avoid unnecessary access
to it during the initialization of vm contexts.
- Remove the Atomics.wake deletion as V8 now does not install it
anymore.
- Move the Intl.v8BreakIterator deletion into the snapshot.
- Do not query the Object prototype if --disable-proto is not set.
This should speed up the creation of vm contexts by about ~12%.
PR-URL: https://github.com/nodejs/node/pull/44252
Refs: https://github.com/nodejs/node/issues/44014
Refs: https://github.com/nodejs/node/issues/37476
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Igor Sheludko