process: split bootstrappers by threads that can run them #21378

joyeecheung · 2018-06-17T17:19:45Z

The first commit is from #21377

This patch split part of the bootstrappers into three files:

lib/internal/process/main_thread_only.js: contains bootstrappers
that can only be run in the main thread, including
- setupStdio for the main thread that sets up process.stdin,
  process.stdout, process.error that may interact with external
  resources, e.g. TTY/File/Pipe/TCP sockets
- setupProcessMethods that setup methods changing process-global
  states, e.g. process.chdir, process.umask, process.setuid
- setupSignalHandlers
- setupChildProcessIpcChannel that setup process.send for
  child processes.
lib/internal/process/worker_thread_only.js: contains bootstrappers
that can only be run in the worker threads, including
- setupStdio for the worker thread that are streams to be
  manipulated or piped to the parent thread
lib/internal/process/per_thread.js: contains bootstrappers
that can be run in all threads, including:
- setupAssert for process.assert
- setupCpuUsage for process.cpuUsage
- setupHrtime for process.hrtime and process.hrtime.bigint
- setupMemoryUsage for process.memoryUsage
- setupConfig for process.config
- setupKillAndExit for process.kill and process.exit
- setupRawDebug for process._rawDebug
- setupUncaughtExceptionCapture for
  process.setUncaughtExceptionCaptureCallback and
  process.hasUncaughtExceptionCaptureCallback

Hopefully in the future we can sort more bootstrappers in
boostrap/node.js into these three files and further group
them into functions that can be run before creating the
snapshot / after loading the snapshot.

This patch also moves most of the isMainThread conditionals
into the main bootstrapper instead of letting them scattered around
special-casing different implementations.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

nodejs-github-bot · 2018-06-17T17:19:47Z

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/87/pipeline

joyeecheung · 2018-06-17T17:27:35Z

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

joyeecheung · 2018-06-20T11:43:58Z

Ping @addaleax if you want to take a look, but it should be pretty safe to land since it's just internal refactoring

This patch split part of the bootstrappers into three files: - `lib/internal/process/main_thread_only.js`: contains bootstrappers that can only be run in the main thread, including - `setupStdio` for the main thread that sets up `process.stdin`, `process.stdout`, `process.error` that may interact with external resources, e.g. TTY/File/Pipe/TCP sockets - `setupProcessMethods` that setup methods changing process-global states, e.g. `process.chdir`, `process.umask`, `process.setuid` - `setupSignalHandlers` - `setupChildProcessIpcChannel` that setup `process.send` for child processes. - `lib/internal/process/worker_thread_only.js`: contains bootstrappers that can only be run in the worker threads, including - `setupStdio` for the worker thread that are streams to be manipulated or piped to the parent thread - `lib/internal/process/per_thread.js`: contains bootstrappers that can be run in all threads, including: - `setupAssert` for `process.assert` - `setupCpuUsage` for `process.cpuUsage` - `setupHrtime` for `process.hrtime` and `process.hrtime.bigint` - `setupMemoryUsage` for `process.memoryUsage` - `setupConfig` for `process.config` - `setupKillAndExit` for `process.kill` and `process.exit` - `setupRawDebug` for `process._rawDebug` - `setupUncaughtExceptionCapture` for `process.setUncaughtExceptionCaptureCallback` and `process.hasUncaughtExceptionCaptureCallback` Hopefully in the future we can sort more bootstrappers in `boostrap/node.js` into these three files and further group them into functions that can be run before creating the snapshot / after loading the snapshot. This patch also moves most of the `isMainThread` conditionals into the main bootstrapper instead of letting them scattered around special-casing different implementations.

joyeecheung · 2018-06-20T12:12:21Z

Rebase since #21377 landed, GitHub said there were conflicts although I applied the commit without any conflicts locally so the patch is still the same (since this PR used to contain a commit from #21377 and after rebase there's only the second commit left).

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

addaleax

Looks good and CI is green :)

Fishrock123

lgtm

joyeecheung · 2018-06-20T16:15:26Z

Landed in e43d91c, thanks!

This patch split part of the bootstrappers into three files: - `lib/internal/process/main_thread_only.js`: contains bootstrappers that can only be run in the main thread, including - `setupStdio` for the main thread that sets up `process.stdin`, `process.stdout`, `process.error` that may interact with external resources, e.g. TTY/File/Pipe/TCP sockets - `setupProcessMethods` that setup methods changing process-global states, e.g. `process.chdir`, `process.umask`, `process.setuid` - `setupSignalHandlers` - `setupChildProcessIpcChannel` that setup `process.send` for child processes. - `lib/internal/process/worker_thread_only.js`: contains bootstrappers that can only be run in the worker threads, including - `setupStdio` for the worker thread that are streams to be manipulated or piped to the parent thread - `lib/internal/process/per_thread.js`: contains bootstrappers that can be run in all threads, including: - `setupAssert` for `process.assert` - `setupCpuUsage` for `process.cpuUsage` - `setupHrtime` for `process.hrtime` and `process.hrtime.bigint` - `setupMemoryUsage` for `process.memoryUsage` - `setupConfig` for `process.config` - `setupKillAndExit` for `process.kill` and `process.exit` - `setupRawDebug` for `process._rawDebug` - `setupUncaughtExceptionCapture` for `process.setUncaughtExceptionCaptureCallback` and `process.hasUncaughtExceptionCaptureCallback` Hopefully in the future we can sort more bootstrappers in `boostrap/node.js` into these three files and further group them into functions that can be run before creating the snapshot / after loading the snapshot. This patch also moves most of the `isMainThread` conditionals into the main bootstrapper instead of letting them scattered around special-casing different implementations. PR-URL: #21378 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

targos · 2018-06-20T19:32:08Z

Should land on v10.x-staging after ~~#21378~~ #21256

joyeecheung · 2018-06-20T19:45:17Z

@targos Did you mean #21377?

targos · 2018-06-20T19:55:30Z

@joyeecheung no, sorry. I meant #21256

targos · 2018-07-14T12:26:06Z

@joyeecheung Previous PRs now landed on v10.x-staging. Would you be willing to backport this one now?

targos · 2018-07-18T05:50:03Z

Ping

This patch split part of the bootstrappers into three files: - `lib/internal/process/main_thread_only.js`: contains bootstrappers that can only be run in the main thread, including - `setupStdio` for the main thread that sets up `process.stdin`, `process.stdout`, `process.error` that may interact with external resources, e.g. TTY/File/Pipe/TCP sockets - `setupProcessMethods` that setup methods changing process-global states, e.g. `process.chdir`, `process.umask`, `process.setuid` - `setupSignalHandlers` - `setupChildProcessIpcChannel` that setup `process.send` for child processes. - `lib/internal/process/worker_thread_only.js`: contains bootstrappers that can only be run in the worker threads, including - `setupStdio` for the worker thread that are streams to be manipulated or piped to the parent thread - `lib/internal/process/per_thread.js`: contains bootstrappers that can be run in all threads, including: - `setupAssert` for `process.assert` - `setupCpuUsage` for `process.cpuUsage` - `setupHrtime` for `process.hrtime` and `process.hrtime.bigint` - `setupMemoryUsage` for `process.memoryUsage` - `setupConfig` for `process.config` - `setupKillAndExit` for `process.kill` and `process.exit` - `setupRawDebug` for `process._rawDebug` - `setupUncaughtExceptionCapture` for `process.setUncaughtExceptionCaptureCallback` and `process.hasUncaughtExceptionCaptureCallback` Hopefully in the future we can sort more bootstrappers in `boostrap/node.js` into these three files and further group them into functions that can be run before creating the snapshot / after loading the snapshot. This patch also moves most of the `isMainThread` conditionals into the main bootstrapper instead of letting them scattered around special-casing different implementations. PR-URL: nodejs#21378 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

This patch split part of the bootstrappers into three files: - `lib/internal/process/main_thread_only.js`: contains bootstrappers that can only be run in the main thread, including - `setupStdio` for the main thread that sets up `process.stdin`, `process.stdout`, `process.error` that may interact with external resources, e.g. TTY/File/Pipe/TCP sockets - `setupProcessMethods` that setup methods changing process-global states, e.g. `process.chdir`, `process.umask`, `process.setuid` - `setupSignalHandlers` - `setupChildProcessIpcChannel` that setup `process.send` for child processes. - `lib/internal/process/worker_thread_only.js`: contains bootstrappers that can only be run in the worker threads, including - `setupStdio` for the worker thread that are streams to be manipulated or piped to the parent thread - `lib/internal/process/per_thread.js`: contains bootstrappers that can be run in all threads, including: - `setupAssert` for `process.assert` - `setupCpuUsage` for `process.cpuUsage` - `setupHrtime` for `process.hrtime` and `process.hrtime.bigint` - `setupMemoryUsage` for `process.memoryUsage` - `setupConfig` for `process.config` - `setupKillAndExit` for `process.kill` and `process.exit` - `setupRawDebug` for `process._rawDebug` - `setupUncaughtExceptionCapture` for `process.setUncaughtExceptionCaptureCallback` and `process.hasUncaughtExceptionCaptureCallback` Hopefully in the future we can sort more bootstrappers in `boostrap/node.js` into these three files and further group them into functions that can be run before creating the snapshot / after loading the snapshot. This patch also moves most of the `isMainThread` conditionals into the main bootstrapper instead of letting them scattered around special-casing different implementations. PR-URL: #21378 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Backport-PR-URL: #21866 Reviewed-By: Michaël Zasso <targos@protonmail.com>

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 17, 2018

joyeecheung added process Issues and PRs related to the process subsystem. worker Issues and PRs related to Worker support. labels Jun 17, 2018

joyeecheung force-pushed the refactor-process-setup branch from 7b83cb1 to dff4560 Compare June 17, 2018 17:25

joyeecheung requested a review from addaleax June 17, 2018 17:26

devsnek approved these changes Jun 17, 2018

View reviewed changes

joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 20, 2018

joyeecheung force-pushed the refactor-process-setup branch from dff4560 to 3a4fc4d Compare June 20, 2018 12:11

addaleax approved these changes Jun 20, 2018

View reviewed changes

Fishrock123 approved these changes Jun 20, 2018

View reviewed changes

joyeecheung closed this Jun 20, 2018

targos added backport-requested-v10.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 20, 2018

mcollina mentioned this pull request Jul 9, 2018

inspector: expose original console #21659

Closed

4 tasks

targos added backport-requested-v10.x and removed backport-requested-v10.x labels Jul 14, 2018

mcollina mentioned this pull request Jul 17, 2018

v10.7.0 proposal #21851

Merged

joyeecheung mentioned this pull request Jul 18, 2018

[v10.x] process: split bootstrappers by threads that can run them #21866

Closed

targos added backported-to-v10.x and removed backport-requested-v10.x labels Jul 18, 2018

Uh oh!

process: split bootstrappers by threads that can run them #21378

process: split bootstrappers by threads that can run them #21378

Uh oh!

Conversation

joyeecheung commented Jun 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Jun 17, 2018

Uh oh!

joyeecheung commented Jun 17, 2018

Uh oh!

joyeecheung commented Jun 20, 2018

Uh oh!

joyeecheung commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Fishrock123 left a comment

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Jun 20, 2018

Uh oh!

targos commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Jun 20, 2018

Uh oh!

targos commented Jun 20, 2018

Uh oh!

targos commented Jul 14, 2018

Uh oh!

targos commented Jul 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joyeecheung commented Jun 17, 2018 •

edited

Loading

joyeecheung commented Jun 20, 2018 •

edited

Loading

targos commented Jun 20, 2018 •

edited

Loading