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

[v11.x] Backport "process: split execution entry points into main scripts" #26036

Closed
wants to merge 2 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Feb 10, 2019

Backport of #25667

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>
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>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v11.x labels Feb 10, 2019
@targos
Copy link
Member Author

targos commented Feb 10, 2019

@targos
Copy link
Member Author

targos commented Feb 10, 2019

The main conflict was in lib/internal/bootstrap/node.js because of the indentation changes. I recreated the diff for that file (only deleted the same lines as the original PR and applied eslint --fix).

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is good

@targos
Copy link
Member Author

targos commented Feb 10, 2019

Landed in e6a4fb6 and 8db6b8a

@targos targos closed this Feb 10, 2019
targos pushed a commit that referenced this pull request Feb 10, 2019
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 pushed a commit that referenced this pull request Feb 10, 2019
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 deleted the backport-25667 branch February 10, 2019 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants