Node: RFC: speeding up Node.js startup using V8 snapshot

[hashseed]

I recently went through Node.js bootstrapping code, and think that we could make it a lot faster using V8 snapshot. I wrote a design doc that captures the main points.

This is somewhat separate from the discussion on using V8 snapshot to capture arbitrary initialized state, discussed here. The main difference is that the set of native modules is known upfront, and there is no ambiguity about the native bindings that need to be known to V8's serializer/deserializer.

I'm doing this as sort of a side project, so it may take some time for me to make progress. Any help is welcome.

[mhdawson]

Sounds great. Is there a repo/branch people can take a look at the changes you have so far for this work ?

[hashseed]

I have a development branch here. All tests still pass, because they are not affected. Running node snapshot attempts to create a snapshot blob. It currently still fails, but I have collected almost all native references. There is still a long way to go though.

[juancampa]

Hi @hashseed, this is amazing and very needed, thanks for working on this. I asked on #17103 but it seems pertinent to this thread (since it's an RFC) so I'll ask here as well.

Would these changes allow for taking snapshots in arbitrary points in time? as opposed to right after startup? If not, do you mind explaining if such a thing would even be feasible. Seems like the guys from Chakra have been working on something like it for Time Travel Debugging (as mentioned by @mrkmarron: https://github.com/nodejs/node/issues/13877#issuecomment-311819701)

To elaborate a little, I'm thinking something like CRIU, where you can send a signal to the process (e.g. kill -USR2 <pid> or some other mechanism) and have the process serialize it's state (to a predefined location maybe) and optionally exit. Then later in time you could restore that serialized state by providing it to a new node process (e.g. node --restore <state-file>)

[hashseed]

It would, once finished, help achieve that goal. Current V8 API is not laid out to serialize arbitrary isolates at arbitrary times, but something in that direction would be thinkable.

However, there are quite a few C++ objects allocated by native modules. Each module would have to provide a way to serialize/deserialize its objects, and we would need a way to dispatch to each module for serialization/deserialization. This hurdle applies to Node with Chakra too.

Another issue is that there is no good way to serialize/deserialize the stack, including C++ frames and local variables that hold onto objects on the JS heap.

[mrkmarron]

Hi @hashseed, I was able to take a quick look at the document and the commits. As you say it looks like there is a lot of work left but this looks like a great start. Please let me know if there is anything that might be useful to help and I will try to make some time. Otherwise, I'll definitely keep an eye on the progress here.

Thanks for this effort!

[hashseed]

Hi Mark,

this is more of a solve-issues-as-we-go approach. Once I have a roughly working version done, there will be some more careful auditing necessary to see which part of initialization needs to be done after snapshotting because they are runtime-dependent, for example gated by command line args. Maybe that's something you can dig into, if you have the time?

[mrkmarron]

Sure, I have looked into this type of thing in the past. The two most common approaches seem to be either (1) making this type of startup a pure programmer responsibility or (2) using symbolic execution (ala prepack). Neither solution is 100% satisfactory but I'll try to spend some time investigating to get a better understanding of things.

[hashseed]

I updated the branch. So far running node snapshot creates a snapshot_blob.bin file into the working directory. With the file in place, running node uses the snapshot, deserializes an isolate and a context, and then immediately quits (because I haven't spent time on making the deserialized context work yet :D).

For some reason I wasn't able to reproduce the 400ms start up for vanilla Node.js anymore, but rather 200ms. I wonder whether I made some mistake with the build. But when I instrumented my branch with uv_hrtime() I was at least able to reproduce 4x speed up.

I'll carry on to make the deserialized context actually work.

[hashseed]

@addaleax @bnoordhuis @TimothyGu is there any reason we store the environment in a v8::External object to pass to function templates as data, instead of getting the current context from the isolate and the environment from there?

v8::Externals are tricky to serialize. They should belong to the context. Function and object templates belong to the isolate, but can own v8::External objects as data. With the current API there is no good way serialize templates as part of the context.

v8::External objects are also kinda hacky. They look like JavaScript objects, but are special in that they are context-independent and don't have a constructor.

[bnoordhuis]

is there any reason we store the environment in a v8::External object to pass to function templates as data, instead of getting the current context from the isolate and the environment from there?

@hashseed See commit 7e88a9322c8, it's explained in the commit log.

[hashseed]

I see. So these functions are fundamentally bound to the context. Would it help if FunctionCallbackInfo can return the function's creation context?

[bnoordhuis]

Yep, that should work if it's also done for PropertyCallbackInfo.

[hashseed]

Quite a few yaks to shave... I think I'll just pile things on this branch first and later disentangle into single PRs.

[hashseed]

@bnoordhuis for PropertyCallbackInfo the Holder should suffice? I don't think there is a way to move an interceptor from one object to another.

As for FunctionCallbackInfo, the corresponding class in v8::internal, FunctionCallbackArguments, actually already has a callee argument in the constructor. It's just never used or stored anywhere. That can be fixed.

Edit: looks like removing callee was intentional. Guess I'll solve it differently: FunctionTemplates and ObjectTemplates that reference some data that is context-dependent (including v8::External) must be serialized as part of the context.

[hashseed]

@bnoordhuis I just verified with @verwaest that inside a function template callback, the current context is indeed already the context of the function being called.

That means we don't need to wrap the environment in a v8::External anymore.

[verwaest-zz]

This isn't true yet for PropertyCallbackInfo and Interceptors though. We could do the same there as we do for "lazy accessor pairs" (function template based accessors).

[liqyan]

Hi hashseed, i have an experimental implemention here. It roughly works.

Just like points you wrote in design doc, i do:

• splitting up node_bootstrap.js's startup into two functions: bootstrap and new startup, for node_mksnapshot, bootstrap is the entry, for node booting from snapshot, the latter is the entry.
• registering external references to SnapshotCreator, All is c++ function, and node::Environment*

and some more:

• support registered Persistent/Global handles serialization
• distinguish Persistent/Global handles from Context aware or not, registering the former to Context, registering the latter to Isolate
• while booting from snapshot, first allocate memory for node::Environment without initialization, register it to Isolate, then using placement new to create node::Environment after Context has been created
• change the behavior of external string serialization. External string will still be external string if it's ExternalStringResource is registered. v8 SnapshotCreator converts external string to internal string. node's builtin modules are all exteranl strings, after booting from snapshot, they are internal strings whose chars allocated in v8 heap. These internal strings can't be shared if node fork/exec or just fork child processes

some remain issues:

• external references registering and c++ function binding looks like redundantly
• setupGlobalConsole consumes significant time ( 14ms, vs total running time 39ms with snapshot on my workstation), but node_mksnapshot can't run with it currently(crashed), node booting from snapshot must restore the status below:
  
  
  
  • one ChannelWrap and one SignalWrap object will be created
  
  
  • two TTYWrap object will be created, and some uv tty operations will be executed
  
  

Running the command time ./node -e "", with snapshot ther result is 39ms, without is 88ms, on my workstation.

howto build

 
./configure --with-node-snapshot
make
cd out/Release
./node_mksnapshot --startup-blob=snapshot_blob.bin

Hope this helps.

[hashseed]

Wow. That's a big surprise. I'll definitely check out your implementation once I have some time.

I must admit that I haven't actually pursued my prototype much further in the last couple of weeks since I haven't had time and had to wrap up a few other projects at the end of the year.

When you say "external references registering and c++ function binding looks like redundantly" do you mean it's not necessary? Is that because you do not set up any function templates when creating the snapshot? Function templates has been one of the biggest blockers for me since they point to the Environment via data pointer, which makes them no longer context-independent.

Cutting the startup time to less than half is a bit less than what I expected, but definitely a significant win! We should try to get this merged, maybe incrementally.

[liqyan]

@hashseed

When you say "external references registering and c++ function binding looks like redundantly" do you mean it's not necessary?

What i want to say is c++ functions binding in node builtin addon initialization procedure, and registering these c++ functions to SnapshotCreator while creating snapshot or to Isolate while booting from snapshot, do similar things, like env->SetMethod(target, name, api_callback), or external_references[idx]=api_callback. If there's a way to do these things in one mechanism, it'd be great.

Cutting the startup time to less than half is a bit less than what I expected, but definitely a significant win! We should try to get this merged, maybe incrementally.

As i said, setupGlobalConsole consumes significant time. Once node_mksnapshot can be executed with it, the startup time will be 1/4.

[hashseed]

I opened an issue on V8 to track implementing serializer/deserializer support for external strings: https://bugs.chromium.org/p/v8/issues/detail?id=7240

I have some other ideas than the one you implemented. I'll work on that and land that upstream.

[hashseed]

I took a look at the changes that affect V8:

1. deps: v8: serialize/deserialize external string table correctly
2. deps: v8: support ser/des global/eternal handles
3. deps: v8: support visit external string

To support external strings, I came up with this upstream change. Does that solve the issue that commits (1) and (3) solve? I'm not entirely sure what commit (1) fixes.

As for commit (2), I think it goes into the right direction, but is a bit over-engineered. You are putting global and eternal handles into fixed arrays before serialization, so that you can re-create global and external handles after deserialization. You distinguish between ones that are context-independent and ones that are context-dependent. A few thoughts on that:

• With this approach, do we really need to distinguish between global and external handles, if they are both stored in fixed arrays?
• Should context-dependent eternal handles even exist in the first place?
• Assuming that we have no context-dependent eternal handles, we could deserialize eternal handles into a std::vector of pointers that we explicitly visit during GC, like the partial-snapshot-cache. Getting external handles from that std::vector would be simple, as that's essentially the same data structure the eternal handle implementation already uses.

I would love if you could contribute (2) to upstream V8. Due to legal reasons I can't just copy your code and submit it upstream.

What i want to say is c++ functions binding in node builtin addon initialization procedure, and registering these c++ functions to SnapshotCreator while creating snapshot or to Isolate while booting from snapshot, do similar things, like env->SetMethod(target, name, api_callback), or external_references[idx]=api_callback. If there's a way to do these things in one mechanism, it'd be great.

Do you mean to say that you would like to have a way to add external references late? How late? At the point of SnapshotCreator::CreateBlob? That should be doable and simply an API change.

[liqyan]

@hashseed

To support external strings, I came up with this upstream change. Does that solve the issue that commits (1) and (3) solve? I'm not entirely sure what commit (1) fixes.

You solved the issue by skipping visiting external string table, much more compact.

The thoughts on commit(2) are very good, i'll refact the part. i'am glad if i could help.

Do you mean to say that you would like to have a way to add external references late? How late? At the point of SnapshotCreator::CreateBlob? That should be doable and simply an API change.

a bit like, we can delay adding external references for serializing, while i haven't figure out a way for deserializing yet.

[hashseed]

Regarding serializing and deserializing global handles, I tried to do this in my prototype without changing the V8 API too much. I ran into limitations though. But maybe there is some ideas there you could reuse:

• For context-dependent global handles, you could simply store them onto the context via v8::Context::SetEmbedderData before serialization. You can restore them via v8::Context::GetEmbedderData after deserialization. This API probably needs some changes because currently you can only store/load v8::Local<v8::Value>, but template objects are excluded from that. A templatized API like you have here would solve this issue.
• For context-independent global handles, I had no good solution, so I think the API you added makes a lot of sense.
• For context-independent eternal handles, I side-stepped the problem since the only eternal handles Node.js seems to use are for string constants, and creating them from scratch should be fairly cheap. But the clean solution would be to make them part of the snapshot, and as I described above, you could deserialize them into a std::vector and expose them as handles directly the same way we do for root list items.

I would very much welcome upstream contributions!

[liqyan]

I have cut the time to 24ms by pre-requiring console in node_mksnapshot. see here

time ./node -e ""
pre-requiring console:          0m0.024s
without pre-requiring console:  0m0.039s
original node:                  0m0.088s

And refact the commit(2) here.
Reusing v8::Context::SetEmbedderData seems a little confusing, it maybe more clearly to add a new field
And as you suggested, it' not necessary to distinguish between persistent and eternal handles, except lifetime control. Objects deserialized from Persistent handles never get reclaimed, as it referenced by serialized_non_local_handles. Maybe we should implement some kind "move semantic" to FromSnapshot. Only first occurrence is valid, then the reference from serialized_non_local_handles release, any subsequent invocation returns empty. I think v8::FunctionTemplate::FromSnapshot has same issue.

[hashseed]

So if I understand correctly, we need ways to:

1. Serialize context-independent eternal handles.
2. Serialize context-independent persistent handles and also a way to release them.
3. Serialize context-dependent persistent handles and also a way to release them.

For (1), I think I can implement the idea I previously described.
For (2) and (3), I think we should just introduce the following APIs:

• void Isolate::SetEmbedderField(v8::Local<T>, int index)
• v8::Local<T> Isolate::GetEmbedderField(int index)
• void Context::SetEmbedderField(v8::Local<T>, int index)
• v8::Local<T> Context::GetEmbedderField(int index)

This way, we would have a more powerful, general API, and the embedder has full control over the lifetime. The move semantics could be implemented by getting the field and then setting it to undefined.

WDYT?

Also the reduction to 24ms sounds very promising!

[hashseed]

@MylesBorins @jasnell I'd like your opinion on this. We are looking at a 4x speed up of Node.js startup here.

This feature requires some changes to V8, which I'm going to implement upstream soon. However, if we wait until these changes end up in Node the usual way, we will have to wait another couple months, and won't have it until Node 10. It won't make sense to look at patches to Node core until then, and during that review, we might need to make further changes to the V8 API.

I would be willing to prepare floating patches to Node master once V8-side changes land upstream to reduce the round-trip. WDYT?

[jasnell]

The general rule of thumb is to wait until such changes are upstreamed. We
can try a PR to float the patches but I'm not sure they will successfully
land.

On Dec 26, 2017 07:14, "Yang Guo" [e-mail] wrote:

@MylesBorins https://github.com/mylesborins @jasnell
https://github.com/jasnell I'd like your opinion on this. We are
looking at a 4x speed up of Node.js startup here.

This feature requires some changes to V8, which I'm going to implement
upstream soon. However, if we wait until these changes end up in Node the
usual way, we will have to wait another couple months, and won't have it
until Node 10. It won't make sense to look at patches to Node core until
then, and during that review, we might need to make further changes to the
V8 API.

I would be willing to prepare floating patches to Node master once V8-side
changes land upstream to reduce the round-trip. WDYT?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/17058#issuecomment-353978825, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2eVtRTLvHRrEKyYNtpCmt6tbmIPyGks5tEQ1KgaJpZM4QffVj
.

[hashseed]

I'm suggesting to land V8-side patches upstream first, then I would prepare these patches so that they can be applied to the version of V8 that Node master currently uses so that they can be floated.

[liqyan]

@hashseed
I think these APIs are too general. It could be misused in some scenarios that the embedder even doesn't try to create snapshot. I suggest introducing APIs to v8::SnapshotCreator to minimize side effect:

Serialize context-dependent persistent handles and also a way to release them.

  void v8::SnapshotCreator::SetDefaultContext(Local context,
                         Data** persistent_handles,
                         SerializeInternalFieldsCallback callback);

  size_t v8::SnapshotCreator::AddContext(Local context,
                         Data** persistent_handles,
                         SerializeInternalFieldsCallback callback);

  MaybeLocal v8::PersistentBase::FromSnapshot(Local context, size_t index);

Serialize context-independent persistent handles and also a way to release them.

  void v8::SnapshotCreator::AddPersistentHandles(Data** persistent_handles);
  MaybeLocal v8::PersistentBase::FromSnapshot(Isolate* isolate, size_t index);

Similar with v8::Isolate::CreateParams::external_references, persistent_handles is a nullptr-terminated array of v8::Data*s, which is the result of dereferencing handles.
The move semantics is done by v8::PersistentBase::FromSnapshot automaticlly, subsequent invocation with the same index will return MaybeLocal<>().

[hashseed]

I wouldn't mind making it too general, especially when we already have less powerful versions of SetEmbedderField and GetEmbedderField.

But I can also see the point in your suggestion. However, I'd rather not use Data**, but add v8::Local one by one, essentially a more general version of v8:: SnapshotCreator::AddTemplate.

I'll then also introduce v8::MaybeLocal v8::Local::FromSnapshot(int index) and v8::MaybeLocal v8::Local::FromSnapshot(v8::Local<v8::Context>, int index).

How does that sound?

[liqyan]

That sounds great.
I prefer a more general version of v8:: SnapshotCreator::AddTemplate too.

And as v8::MaybeLocal v8::Local::FromSnapshot(int index), would it be better if we use the from v8::MaybeLocal<T> v8::Local::FromSnapshot(v8::Isolate* isolate, size_t index) and v8::MaybeLocal<T> v8::Local::FromSnapshot(v8::Local<v8::Context>, size_t index)?

[liqyan]

I summarize APIs we need:

• Add handle to v8::SnapshotCreator while creating snapshot:

size_t v8::SnapshotCreator::AddData(v8::Local<T> data);
size_t v8::SnapshotCreator::AddData(v8::Local<v8::Context> context, v8::Local<T> data);

• Get handle from snapshot

v8::MaybeLocal<T> v8::Local::FromSnapshot(v8::Isolate* isolate, size_t index);
v8::MaybeLocal<T> v8::Local::FromSnapshot(v8::Local<v8::Context> context, size_t index);

But these objects still referenced by internal data, so i think we should use the following form, release the reference automaticly.

• Get handle from snapshot and release

v8::MaybeLocal<T> v8::Local::FromSnapshotAndRelease(v8::Isolate* isolate, size_t index);
v8::MaybeLocal<T> v8::Local::FromSnapshotAndRelease(v8::Local<v8::Context> context, size_t index);

What do you think?

[hashseed]

Another way is to have these methods on Isolate and Context instead, which I slightly prefer, since that leaves v8::Local untouched.

Something like Isolate::GetDataFromSnapshotAndRelease or Isolate:: TakeDataFromSnapshot?

I won't actually be able to work on this until second week of 2018 due to vacation, but this doesn't sound complicated.

[liqyan]

So we got these:

v8::MaybeLocal<T> v8::Isolate::GetDataFromSnapshotAndRelease(size_t index);
v8::MaybeLocal<T> v8::Context::GetDataFromSnapshotAndRelease(size_t index);

Once the APIs are OK I will submit the code.

Wish you a happy holiday.

[hashseed]

External string support has been implemented here.
Support for arbitrary additional data has been implemented here.

Is there anything else missing?

I attempted to back port these changes to node master, but there were quite a few merge conflicts. The V8 version on node master also does not (correctly) support serializing typed arrays. So maybe we can develop on a fork of v8/node/vee-eight-lkgr for the time being? I can retry back porting once node master upgrades to V8 6.4.

[bnoordhuis]

We're waiting for 6.4 to go stable on January 23, see #17489. You could float your changes on top of that PR.

[hashseed]

@liqyan Can I ask you to check in on your progress? I've seen you submit patches to both V8 and Node.js from time to time, but I don't have an overview. If there is anything stalling your progress or if you need support, please reach out!

[liqyan]

Hey, I rewrote the experimental implementation with the new snapshot APIs here.

All is fine except those command line arguments passed to node and system environment variables. Node mixes node initialization and running user code together. And during node initialization, we have to test those command line arguments and system environment variables and paths, which maybe not consistent between creating snapshot and using snapshot.

To solve this problem, I'm writing a proposal that we should use a explicitly initialization procedure, and this procedure use nothing that maybe not consistent between creating snapshot and using snapshot.
I'm about to finish the proposal.

[hashseed]

Great to see you are making progress!

I do agree that Node.js' bootstrapping is not ideal and by far not as isolated as it should be. I'm looking forward to the proposal and will take a look at your current experimental implementation in the meantime.

[liqyan]

Problem

Currently lib/internal/bootstrap_node.js looks like this:

(function(process) {
 function startup() {
  // node booting
  // test arguments and run user code
 }

 ...

 startup();
});

There's no clear boundary between node booting and running user code. And the state that node bootstrap finished varies due to runtime arguments (command line arugments, system environment variables, etc.).

To use snapshot to speed up node startup, we have to make sure there's a consistent state between creating snapshot and using snapshot. This means we have to eliminate all the effects of runtime arguments.

Proposal

To achive the consistent state, this proposal suggests node should have boot procedure like this:

(function(process) {
   function setupNode() {
     // run things that don't depend on runtime arguments

    ...

    preloadModules();
    return startup;
  }

  var postpones = [];
  function postSetupNode() {
    // run things that depend on runtime arguments
    // invoke each function in postpones and clear it
  }

  function startup() {
    postSetupNode();

    ...

    // test arguments and run user code
  }

  ...

  return setupNode();
});

setupNode and postSetupNode establish the javascript runtime together.
The main criteria is setupNode must not access runtime arguments directly or indirectly. This means:

• Native module reqiured by setupNode must not access runtime arguments during "require" directly or indirectly. If we support preloadModules while creating snapshot, it means all native modules.
• setupNode must not invoke functions exported by native modules.
• setupNode must not invoke functions defined in lib/internal/bootstrap_node.js.

Native Modules

To satisfy criteria 1 we have to refact native modules like this:

'usr strict'

// do init things
// do export

maybe_postpone_require(() => {
  // things access runtime arguments directly or indirectly in previous top level function
});

maybe_postpone_require is accessable via NativeModule.wrapper. It postpones functions passed to it until postSetupNode when executing setupNode, and invokes functions passed to it immediately then.

  let postponed_fns = [];
  let maybe_postpone_require = function(postponed_fn) {
    postponed_fns.push(postponed_fn);
  }

  function postSetupNode() {
    // reset maybe_postpone_require
    maybe_postpone_require = function(postponed_fn) {
      postponed_fn();
    }

    postponed_fns.forEach(function(fn) {
      fn();
    });

    postponed_fns = undefined;
  }

  NativeModule.wrapper = [
    '(function (exports, require, module, internalBinding, maybe_postpone_require, process) {',
    '\n});'
  ];

Only native modules are involved since user modules use another version require.

Split up setup

To satisfy criteria 2 and 3, we have to split those setup functions into 2 parts: one part that can be invoked by setupNode, the rest will be invoked by postSetupNode.

Binding addons

Some initialization funcition of builtin addons do more thing besides setting up the addon object, just look at crypto:

void InitCrypto(Local<Object> target,
                Local<Value> unused,
                Local<Context> context,
                void* priv) {
  static uv_once_t init_once = UV_ONCE_INIT;
  uv_once(&init_once, InitCryptoOnce);

  ...

}

We should register these things while creating snapshot, and run them while boot from snapshot. I'll explain this in another proposal about the c++ part.

Check access violation

All accesses to runtime arguments are via process object(?). So we can set up interceptor for process object to check access violation during booting.

[liqyan]

Struggling with english language, I'm afraid I have not expressed myself clearly. Please do tell me if you don't understand.

[hashseed]

I definitely agree with this proposal. Currently, Node.js startup includes parts that depend on command line flags or env vars, and some internal modules allocate C++ objects.

These have to be factored out so that we have a phase of initialization that initializes Node.js in a deterministic way, followed by a phase that is affected by command line flags and env vars. This would mirror what we have in V8.

I'm not entirely sure wrt C++ objects. If they are referenced by V8 objects via embedder fields, we can serialize them, as you have it in your experimental branch.

[hashseed]

I just brought this up at the V8/TSC call. @jasnell mentioned that he has some plans/ideas to disentangle bootstrapping. James, do you have any specifics that you want to share? I guess this is something we could already get started with even before the necessary V8 API becomes available on master. I would definitely be willing to help.

[jasnell]

Yeah, the key issue is that node.js startup is a Tangled mess of spaghetti code right now. From the way cli args and env vars are processed, to the order of start up events, etc. I've been looking at introducing a proper command line args parser and start up options isolation that will be tied to the Environment rather than using global state, then will be separating out the initialization of the process object. Once that is done, we should be able to refactor the bootstrap script into distinct phases. The key challenge with the snapshot is that any snapshot is going to be tightly coupled with cli/env args at startup if we do it right now so separating these out first will be critical.

[liqyan]

@hashseed

I'm not entirely sure wrt C++ objects. If they are referenced by V8 objects via embedder fields, we can serialize them, as you have it in your experimental branch.

I don't think we have to concern wrt C++ objects. There're 4 wrt C++ objects alive when node complete bootstrap, all others live for temporary. These 4 objects are:

• 2 TTYWrap objects associate with stdout and stderr
• 1 SignalWrap object listens signal 'SIGWINCH' of tty
• 1 ChannelWrap object is defaultResolver in lib/dns.js which is not used during bootstrap

Since we don't need to open tty during bootstrap, there will be no wrt C++ objects once refactor is done.

@hashseed

I guess this is something we could already get started with even before the necessary V8 API becomes available on master. I would definitely be willing to help.

@jasnell

I've been looking at introducing a proper command line args parser and start up options isolation that will be tied to the Environment rather than using global state, then will be separating out the initialization of the process object. Once that is done, we should be able to refactor the bootstrap script into distinct phases.

I think we could refactor the bootstrap script simultaneously, by setting up interceptor to process object for checking runtime arguments access. See here

[liqyan]

Status update:
Most native modules used during bootstrap have been modified that those procedures that accessing runtime arguments have been postponed until runtime arguments accessing is allowed.

Most tests passed except some message tests that strictly compare exception stacks, and some tests that treating native module as normal module by using relative path, such as require('../../lib/buffer').

code is here

[jasnell]

Status update on this side: I've started working on refactoring the native bootstrap lifecycle.... starting with how configuration and command line options are handled.... specifically, moving away from use of global variables to using a NodeOptions class associated with the Environment. From there, I will be working through a refactoring of how the process object is bootstrapped.

[liqyan]

Hi @jasnell, would you please update the status of working on refactoring native bootstrap?

[hashseed]

This seems to be blocked by current bootstrapping code being incompatible with snapshotting. I think we have a chicken-egg problem here. Without clean bootstrapping code, we don't get snapshotting. Without snapshotting, there is little incentive to clean up bootstrapping. Though I think there has been some progress with refactorings.

How about we approach this incrementally. I think snapshotting right after lib/internal/bootstrap/loaders.js might already work?

[devsnek]

@hashseed can you come up with a sort of action item list that contributors can use to figure out what changes need to be made? I assume it's along the lines of separating loading all our code from stuff like checking cli arguments and such?

[hashseed]

I guess a list would look something like this:

• [ ] Collect v8::FunctionCallback, external string resources, etc. into a null-terminated intptr_t* array that can be passed to v8::SnapshotCreator. This would look like this from @liqyan's prototype.
• [ ] Implement persisting and restoring eternal handles from the snapshot, like here.
• [ ] Implement a new build step to create the snapshot. Node.js would run up to bootstrapping and then capture a snapshot. This could either be between executing two scripts, e.g. lib/internal/bootstrap/loaders.js and lib/internal/bootstrap/node.js, or wrapping bootstrapping in an async function so that at the point where we create the snapshot, there is no V8 frames on the stack.
• [ ] Make sure that before snapshotting, we only perform steps that are independent from CLI flags and environment variables. Ideally, we would move as much as possible to before snapshotting, but this could be done incrementally. The proposal laid out by @liqyan looks reasonable to me.
• [ ] Implement a way for Node.js bootstrapping to continue from deserializing from a context.
• [ ] Implement a way to serialize and deserialize C++ objects referenced by the snapshot, like done here.

[joyeecheung]

We could also go over the list produced by running process._rawDebug(process.moduleLoadList.join('\n')) and try not to load unnecessary modules during bootstrapping. For example there are a lot of modules that seem to be irrelevant to bootstrapping in there (e.g. dns, tcp), most of them are eagerly loaded because of the instantiation of the global console and we are not really careful about how we load the built-in modules in the built-in modules. Even acorn was loaded during bootstrapping before https://github.com/nodejs/node/pull/19863

[hashseed]

With snapshotting preloading modules becomes advantageous because bootstrapping becomes a lot cheaper.

It can't hurt to have a list of FunctionCallbacks already.

[devsnek]

sorta a sidenote... with this separation i wonder if we can somehow also mark all the functions we create before user code runs as native for toString purposes

[hashseed]

sorta a sidenote... with this separation i wonder if we can somehow also mark all the functions we create before user code runs as native for toString purposes

That's not exposed, but could easily be implemented. All you need to do is to mark script type as v8::internal::Script::TYPE_NATIVE.

Last time I brought this up I was told that exposing the source of core libs is intended. And I agree that it is useful.

[Trott]

Is this an issue that should remain open? Or is this close-able at this time?

[jasnell]

This is a slow going one but it's still active (even if it doesn't appear to be ;)...). I'll leave it up to you on whether the issue should remain open

[Trott]

This is a slow going one but it's still active (even if it doesn't appear to be ;)...). I'll leave it up to you on whether the issue should remain open

That sounds like it should be left open for now.

[Trott]

(How do we know when this is complete and can be closed?)

[hashseed]

We would be done once we use snapshots for Node.js start up :)

[joyeecheung]

Continued from https://github.com/nodejs/node/issues/27196

I think I've completed

Implement a new build step to create the snapshot. Node.js would run up to bootstrapping and then capture a snapshot. This could either be between executing two scripts, e.g. lib/internal/bootstrap/loaders.js and lib/internal/bootstrap/node.js, or wrapping bootstrapping in an async function so that at the point where we create the snapshot, there is no V8 frames on the stack.

and I am currently working on another iteration that integrates snapshot during the main Environment creation (worker environment is a bit different and it would result in a different blob of snapshot).

Regarding

Collect v8::FunctionCallback, external string resources, etc. into a null-terminated intptr_t* array that can be passed to v8::SnapshotCreator. This would look like this from @liqyan's prototype.

in https://github.com/nodejs/node/issues/17058#issuecomment-381179675 , the prototype essentially copy-pastes all the external references accessed by internal bindings during bootstrap into a bunch of *RegisterExternalReferences() functions that accompany *::Initialize() to register them, but if anyone adds a new external reference without registering it in the binding, it would be a bit difficult to catch the bug. IIUC, in V8 this is checked with a magic number that is constexpr-computed using the same macros that are used to register builtins, in Node we do not have this and everything is registered in a rather ad-hoc way. I am not sure if this maintainability risk is a blocker for us, I can think of a few follow-ups to improve the developer experience though.

[hashseed]

I think serialization will fail if there is an unknown external reference. And we have a command line flag to attempt to symbolize and print these (I forgot which flag though).

[joyeecheung]

Update: I think we can already snapshot the context after running the per-context scripts, because at that point the only external reference that needs to be registered is OnMessage. I will try opening a PR with that minimal snapshot soon, WIP is here.

About OnMessage: it's added by AddMessageListenerWithErrorLevel in SetIsolateUpForNode, among many other per-isolate callbacks, but somehow this is the only one that results in the creation of a Foreign external reference and needs to be registered. At the moment it should be theoretically possible to delay SetIsolateUpForNode() until after bootstrap when generating the snapshot, but since we don't have a proper exit routine yet, we rely on OnMessage to handle any uncaught JS error thrown during bootstrap for development (instead of outright crashing). For now I think it's fine to hard-code this only external reference for the minimal snapshot, but we should probably figure out a smarter way to deal with this..

[joyeecheung]

Turns out simply registering the message handler is not enough, it seems to have certain assumptions that fail ~10 of the tests, mostly with the async id stack corrupted and seems to be related to domains. Delaying the handler setup until after context setup and snapshot before that already works and passes all the tests (with a 5% speedup in the process/semicolon case of the misc/startup benchmark), so I guess we could start minimally for now. OnMessage in the context setup phase has been broken before https://github.com/nodejs/node/pull/27236 anyway.

[hashseed]

Let me know if you need additional features for the snapshot creator API.

[joyeecheung]

I opened https://github.com/nodejs/node/pull/27321 as a first step.

[joyeecheung]

https://github.com/nodejs/node/pull/27321 broke pummel/test-hash-seed (temporarily disabled in https://github.com/nodejs/node/pull/27365), we probably need an API for doing what --rehash-snapshot does in v8 (blocked on https://bugs.chromium.org/p/v8/issues/detail?id=6593 ?)

EDIT: I think it's because we create WeakMap and Map instances in per_context/domexception which renders the snapshot non-rehashable, it would be nice if that is returned by CreateBlob() so that we could CHECK in the build step though.

EDIT: I think my guess was correct. Follow-up in https://github.com/nodejs/node/pull/27371 and https://chromium-review.googlesource.com/c/v8/v8/+/1578661 (I am not sure if the latter is the right way to go about this though). I am also not sure how far the bootstrap can go without Map or Set, though my guess is we could pay a bit of performance hit and replace those with objects and arrays

[hashseed]

The alternative is to make these data structures rehashable.

[styfle]

Enabling V8 snapshots by default landed in #28181 released as Node 12.5.0 🎉

[alubbe]

Does anyone have any benchmarks on this? That PR didn't mention roughly how much impact this could have, for example for CLI tools or for short one liners using node -p '...'

[chocolateboy]

I haven't spotted a difference yet (I assume it's the basis for future improvements) e.g. with:

• OS: Linux (Arch)
• nvm: 0.34.0
• startup-time: 1.1.0

I get:

v12.4.0

$ startup-time --only node --rounds 1000

|Test |Time (ms)|
|-------|-------------|
|JavaScript (Node.js)|36.57|

v12.5.0

$ startup-time --only node --rounds 1000

|Test|Time (ms)|
|------|-------------|
|JavaScript (Node.js)|34.74|

[joyeecheung]

The current snapshot shipped is the minimal one, there were some numbers in https://github.com/nodejs/node/pull/27321#issuecomment-485090927

I am still working on including more of the bootstrap process into the snapshot (which is now handily split into two environment-independent bootstrap scripts). It’s been progressing somewhat slowlier since I have been focusing on something else in the past month. The refactoring of environment dependency has been basically completed (and guarded with assertions now).

The rest of the work comes down to hooking into the V8 API for external references, but we probably need to figure out a way to track and register them all without hard coding everything - I have been hard coding them in my prototype like what was done in the prototype of @liqyan but it seems to be a big maintainance burden to me if it’s not somehow automated. Basically what I have been doing is running the snapshot creator , symbolicating any unknown external references that v8 crashes with, and hard coding the registration of those references, which is not going to be nice for future contributors who don’t know much about the snapshot creator and just want to add bindings in their patches.

[kabirbaidhya]

Hi guys, any recent updates on this thread?

I see the issue is open - does that mean the work is still in-progress? If so, which release does it target?

[joyeecheung]

@kabirbaidhya The complete integration - without regressing on perf and security - currently blocks on a few changes in V8 (e.g. rehashing collections, faster context-independent embedder slots in objects), I am working on getting them resolved in the upstream to unblock, but will take some time to figure these out.

[lolobug]

@kabirbaidhya The complete integration - without regressing on perf and security - currently blocks on a few changes in V8 (e.g. rehashing collections, faster context-independent embedder slots in objects), I am working on getting them resolved in the upstream to unblock, but will take some time to figure these out.

Is there a roadmap？ Speed is a significant factor for my project applying Node.js in Android device.

[joyeecheung]

@lolobug Rehashing has landed in the upstream https://chromium-review.googlesource.com/c/v8/v8/+/2143983 and the object slot issue is less of a concern after https://github.com/nodejs/node/pull/33139.

There are two different PRs for including more of bulitin snapshot (that is, snapshotting the runtime-independent part and speed up the startup universally for all users) into Node.js with different designs https://github.com/nodejs/node/pull/32761 and https://github.com/nodejs/node/pull/32984, and we'll need to address the design differences before this can move forward to user land snapshotting (for building snapshots of their own applications).

You can find out more about the differences in https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit?usp=sharing - feedback is welcomed :)

[joyeecheung]

Since the core bootstrap is now in the builtin snapshot, and we are working towards including more of the bootstrap into the snapshot, maybe it's time to close this and open a new tracking issue?

[gengjiawen]

Since the core bootstrap is now in the builtin snapshot, and we are working towards including more of the bootstrap into the snapshot, maybe it's time to close this and open a new tracking issue?

LGTM.

[joyeecheung]

Opened https://github.com/nodejs/node/issues/35711 for discussions around future snapshot work.