Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: split execution entry points into main scripts #25667

Closed
wants to merge 3 commits into from

Conversation

@joyeecheung
Copy link
Member

commented Jan 23, 2019

Note: there are some indentation changes in lib/internal/bootstrap/node.js so please use https://github.com/nodejs/node/pull/25667/files?w=1 for reviews

process: split execution into main scripts

This patch splits the execution mode selection from the environment
setup in lib/internal/bootstrap/node.js, and split the entry point
of different execution mode into main scripts under
lib/internal/main:

  • check_syntax.js: used when -c/--check which only checks the
    syntax of the input instead of executing it.
  • eval_stdin.js: used when -e is passed without value and stdin
    is not a TTY (e.g. something is piped).
  • eval_string: used when -e is passed along with a string argument
  • inspect.js: for node inspect/node debug
  • print_bash_completion.js: for --completion-bash
  • print_help.js: for --help
  • prof_process.js: for --prof-process
  • repl.js: for the REPL
  • run_main_module.js: used when a main module is passed
  • run_third_party_main.js: for the legacy _third_party_main.js
    support
  • worker_thread.js: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

  • lib/internal/per_context.js to setup the v8 Context (which is
    also run when setting up contexts for the vm module)
  • lib/internal/bootstrap/loaders.js to set up internal binding
    and builtin module loaders (that are separate from the loaders
    accessible in the user land).
  • lib/internal/bootstrap/node.js: to set up the rest of the
    environment, including various globals and the process object
  • lib/internal/main/run_main_module.js: which is selected from
    C++ to prepare execution of the user module.

This patch also removes NativeModuleLoader::CompileAndCall and
exposes NativeModuleLoader::LookupAndCompile directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.

worker: move worker thread setup code into the main script

This patch directly inlines createMessageHandler() and
createWorkerFatalExeception() in the new
lib/internal/main/worker_thread.js since the implementation
of the two methods are related to the execution flow of
workers.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Jan 23, 2019

@joyeecheung joyeecheung requested a review from addaleax Jan 23, 2019

@joyeecheung joyeecheung changed the title process: split execution into main scripts process: split execution entry points into main scripts Jan 23, 2019

src/env.cc Outdated
@@ -295,6 +295,17 @@ void Environment::Start(const std::vector<std::string>& args,
HandleScope handle_scope(isolate());
Context::Scope context_scope(context());

if (args.size() > 1) {
std::string first_arg = args[1];
if (first_arg == "inspect") {

This comment has been minimized.

Copy link
@joyeecheung

joyeecheung Jan 23, 2019

Author Member

Ideally this should not have been done here - maybe a better place would be the options parser but at that point Environment is not yet available and we do not have a better place to carry this information yet.

src/node_options.cc Outdated Show resolved Hide resolved
@addaleax addaleax referenced this pull request Jan 23, 2019
2 of 2 tasks complete
src/node.cc Outdated Show resolved Hide resolved

@joyeecheung joyeecheung force-pushed the joyeecheung:split-execution branch from 0bab6b0 to e5ddc18 Jan 29, 2019

@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

I've removed the options changes as those are not really related to the primary purpose of this PR. @addaleax PTAL.

CI: https://ci.nodejs.org/job/node-test-pull-request/20406/

joyeecheung added 3 commits Jan 15, 2019
process: split execution into main scripts
This patch splits the execution mode selection from the environment
setup in `lib/internal/bootstrap/node.js`, and split the entry point
of different execution mode into main scripts under
`lib/internal/main`:

- `check_syntax.js`: used when `-c`/`--check` which only checks the
  syntax of the input instead of executing it.
- `eval_stdin.js`: used when `-e` is passed without value and stdin
  is not a TTY (e.g. something is piped).
- `eval_string`: used when `-e` is passed along with a string argument
- `inspect.js`: for `node inspect`/`node debug`
- `print_bash_completion.js`: for `--completion-bash`
- `print_help.js`: for `--help`
- `prof_process.js`: for `--prof-process`
- `repl.js`: for the REPL
- `run_main_module.js`: used when a main module is passed
- `run_third_party_main.js`: for the legacy `_third_party_main.js`
  support
- `worker_thread.js`: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

- `lib/internal/per_context.js` to setup the v8 Context (which is
  also run when setting up contexts for the `vm` module)
- `lib/internal/bootstrap/loaders.js` to set up internal binding
  and builtin module loaders (that are separate from the loaders
  accessible in the user land).
- `lib/internal/bootstrap/node.js`: to set up the rest of the
  environment, including various globals and the process object
- `lib/internal/main/run_main_module.js`: which is selected from
  C++ to prepare execution of the user module.

This patch also removes `NativeModuleLoader::CompileAndCall` and
exposes `NativeModuleLoader::LookupAndCompile` directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.
worker: move worker thread setup code into the main script
This patch directly inlines `createMessageHandler()` and
`createWorkerFatalExeception()` in the new
`lib/internal/main/worker_thread.js` since the implementation
of the two methods are related to the execution flow of
workers.

@joyeecheung joyeecheung force-pushed the joyeecheung:split-execution branch from e5ddc18 to 455c923 Jan 29, 2019

@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

Landed in feebdc5...5e1d446

joyeecheung added a commit that referenced this pull request Jan 30, 2019
process: split execution into main scripts
This patch splits the execution mode selection from the environment
setup in `lib/internal/bootstrap/node.js`, and split the entry point
of different execution mode into main scripts under
`lib/internal/main`:

- `check_syntax.js`: used when `-c`/`--check` which only checks the
  syntax of the input instead of executing it.
- `eval_stdin.js`: used when `-e` is passed without value and stdin
  is not a TTY (e.g. something is piped).
- `eval_string`: used when `-e` is passed along with a string argument
- `inspect.js`: for `node inspect`/`node debug`
- `print_bash_completion.js`: for `--completion-bash`
- `print_help.js`: for `--help`
- `prof_process.js`: for `--prof-process`
- `repl.js`: for the REPL
- `run_main_module.js`: used when a main module is passed
- `run_third_party_main.js`: for the legacy `_third_party_main.js`
  support
- `worker_thread.js`: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

- `lib/internal/per_context.js` to setup the v8 Context (which is
  also run when setting up contexts for the `vm` module)
- `lib/internal/bootstrap/loaders.js` to set up internal binding
  and builtin module loaders (that are separate from the loaders
  accessible in the user land).
- `lib/internal/bootstrap/node.js`: to set up the rest of the
  environment, including various globals and the process object
- `lib/internal/main/run_main_module.js`: which is selected from
  C++ to prepare execution of the user module.

This patch also removes `NativeModuleLoader::CompileAndCall` and
exposes `NativeModuleLoader::LookupAndCompile` directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.

PR-URL: #25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
joyeecheung added a commit that referenced this pull request Jan 30, 2019
worker: move worker thread setup code into the main script
This patch directly inlines `createMessageHandler()` and
`createWorkerFatalExeception()` in the new
`lib/internal/main/worker_thread.js` since the implementation
of the two methods are related to the execution flow of
workers.

PR-URL: #25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

@joyeecheung This needs a manual backport for v11.x, it seems.

antsmartian added a commit to antsmartian/node that referenced this pull request Feb 10, 2019
process: split execution into main scripts
This patch splits the execution mode selection from the environment
setup in `lib/internal/bootstrap/node.js`, and split the entry point
of different execution mode into main scripts under
`lib/internal/main`:

- `check_syntax.js`: used when `-c`/`--check` which only checks the
  syntax of the input instead of executing it.
- `eval_stdin.js`: used when `-e` is passed without value and stdin
  is not a TTY (e.g. something is piped).
- `eval_string`: used when `-e` is passed along with a string argument
- `inspect.js`: for `node inspect`/`node debug`
- `print_bash_completion.js`: for `--completion-bash`
- `print_help.js`: for `--help`
- `prof_process.js`: for `--prof-process`
- `repl.js`: for the REPL
- `run_main_module.js`: used when a main module is passed
- `run_third_party_main.js`: for the legacy `_third_party_main.js`
  support
- `worker_thread.js`: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

- `lib/internal/per_context.js` to setup the v8 Context (which is
  also run when setting up contexts for the `vm` module)
- `lib/internal/bootstrap/loaders.js` to set up internal binding
  and builtin module loaders (that are separate from the loaders
  accessible in the user land).
- `lib/internal/bootstrap/node.js`: to set up the rest of the
  environment, including various globals and the process object
- `lib/internal/main/run_main_module.js`: which is selected from
  C++ to prepare execution of the user module.

This patch also removes `NativeModuleLoader::CompileAndCall` and
exposes `NativeModuleLoader::LookupAndCompile` directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.

PR-URL: nodejs#25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
antsmartian added a commit to antsmartian/node that referenced this pull request Feb 10, 2019
worker: move worker thread setup code into the main script
This patch directly inlines `createMessageHandler()` and
`createWorkerFatalExeception()` in the new
`lib/internal/main/worker_thread.js` since the implementation
of the two methods are related to the execution flow of
workers.

PR-URL: nodejs#25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos added a commit to targos/node that referenced this pull request Feb 10, 2019
process: split execution into main scripts
This patch splits the execution mode selection from the environment
setup in `lib/internal/bootstrap/node.js`, and split the entry point
of different execution mode into main scripts under
`lib/internal/main`:

- `check_syntax.js`: used when `-c`/`--check` which only checks the
  syntax of the input instead of executing it.
- `eval_stdin.js`: used when `-e` is passed without value and stdin
  is not a TTY (e.g. something is piped).
- `eval_string`: used when `-e` is passed along with a string argument
- `inspect.js`: for `node inspect`/`node debug`
- `print_bash_completion.js`: for `--completion-bash`
- `print_help.js`: for `--help`
- `prof_process.js`: for `--prof-process`
- `repl.js`: for the REPL
- `run_main_module.js`: used when a main module is passed
- `run_third_party_main.js`: for the legacy `_third_party_main.js`
  support
- `worker_thread.js`: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

- `lib/internal/per_context.js` to setup the v8 Context (which is
  also run when setting up contexts for the `vm` module)
- `lib/internal/bootstrap/loaders.js` to set up internal binding
  and builtin module loaders (that are separate from the loaders
  accessible in the user land).
- `lib/internal/bootstrap/node.js`: to set up the rest of the
  environment, including various globals and the process object
- `lib/internal/main/run_main_module.js`: which is selected from
  C++ to prepare execution of the user module.

This patch also removes `NativeModuleLoader::CompileAndCall` and
exposes `NativeModuleLoader::LookupAndCompile` directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.

PR-URL: nodejs#25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos added a commit to targos/node that referenced this pull request Feb 10, 2019
worker: move worker thread setup code into the main script
This patch directly inlines `createMessageHandler()` and
`createWorkerFatalExeception()` in the new
`lib/internal/main/worker_thread.js` since the implementation
of the two methods are related to the execution flow of
workers.

PR-URL: nodejs#25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos added a commit that referenced this pull request Feb 10, 2019
process: split execution into main scripts
This patch splits the execution mode selection from the environment
setup in `lib/internal/bootstrap/node.js`, and split the entry point
of different execution mode into main scripts under
`lib/internal/main`:

- `check_syntax.js`: used when `-c`/`--check` which only checks the
  syntax of the input instead of executing it.
- `eval_stdin.js`: used when `-e` is passed without value and stdin
  is not a TTY (e.g. something is piped).
- `eval_string`: used when `-e` is passed along with a string argument
- `inspect.js`: for `node inspect`/`node debug`
- `print_bash_completion.js`: for `--completion-bash`
- `print_help.js`: for `--help`
- `prof_process.js`: for `--prof-process`
- `repl.js`: for the REPL
- `run_main_module.js`: used when a main module is passed
- `run_third_party_main.js`: for the legacy `_third_party_main.js`
  support
- `worker_thread.js`: for workers

This makes the entry points easier to navigate and paves the way
for customized v8 snapshots (that do not need to deserialize
execution mode setup) and better embedder APIs.

As an example, after this patch, for the most common case where
Node.js executes a user module as an entry point, it essentially
goes through:

- `lib/internal/per_context.js` to setup the v8 Context (which is
  also run when setting up contexts for the `vm` module)
- `lib/internal/bootstrap/loaders.js` to set up internal binding
  and builtin module loaders (that are separate from the loaders
  accessible in the user land).
- `lib/internal/bootstrap/node.js`: to set up the rest of the
  environment, including various globals and the process object
- `lib/internal/main/run_main_module.js`: which is selected from
  C++ to prepare execution of the user module.

This patch also removes `NativeModuleLoader::CompileAndCall` and
exposes `NativeModuleLoader::LookupAndCompile` directly so that
we can handle syntax errors and runtime errors of bootstrap
scripts differently.

PR-URL: #25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-PR-URL: #26036
targos added a commit that referenced this pull request Feb 10, 2019
worker: move worker thread setup code into the main script
This patch directly inlines `createMessageHandler()` and
`createWorkerFatalExeception()` in the new
`lib/internal/main/worker_thread.js` since the implementation
of the two methods are related to the execution flow of
workers.

PR-URL: #25667
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-PR-URL: #26036

@targos targos added this to Backported in v11.x Feb 10, 2019

@targos targos referenced this pull request Feb 14, 2019
nitsakh added a commit to electron/node that referenced this pull request Mar 27, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
nitsakh added a commit to electron/node that referenced this pull request Mar 28, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 10, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 10, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 10, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 10, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 10, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 11, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
nitsakh added a commit to electron/node that referenced this pull request Apr 11, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
nitsakh added a commit to electron/node that referenced this pull request Apr 11, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
nitsakh added a commit to electron/node that referenced this pull request Apr 11, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
nitsakh added a commit to electron/node that referenced this pull request Apr 11, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 16, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
zcbenz added a commit to electron/node that referenced this pull request Apr 16, 2019
fixme: Add back NativeModuleLoader CompileAndCall
Node removed the NativeModuleLoader CompileAndCall function and exposed
the LookupAndCompile function, with the intention of giving the ability
to the caller to handle errors. The node PR made the changes
nodejs/node#25667.
I'm adding this back for now, but this can easily be moved to electron
as a helper or can probably be upstreamed somehow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.