FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: set up process.nextTick and promise handlers with internalBinding('task_queue') by joyeecheung · Pull Request #25163 · nodejs/node · GitHub

/ node Public

src: set up process.nextTick and promise handlers with internalBinding('task_queue') - #25163

Closed
joyeecheung wants to merge 3 commits into
nodejs:masterfrom
joyeecheung:task-queue
ConversationCommits (3)ChecksFiles changed
Closed

src: set up process.nextTick and promise handlers with internalBinding('task_queue')#25163
joyeecheung wants to merge 3 commits into
nodejs:masterfrom
joyeecheung:task-queue

Conversation

joyeecheung commented Dec 21, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Please use https://github.com/nodejs/node/pull/25163/files?w=1 to skip indentation changes

src: move symbols binding into node_symbols.cc

src: move process.nextTick and promise setup into node_task_queue.cc

This patch:

  • Moves the process.nextTick and promise setup C++ code into
    node_task_queue.cc which is exposed as
    internalBinding('task_queue')
  • Makes lib/internal/process/promises.js and
    lib/internal/process/next_tick.js as side-effect-free
    as possible
  • Removes the bootstrapper object being passed into
    bootstrap/node.js, let next_tick.js and promises.js
    load whatever they need from internalBinding('task_queue')
    instead.
  • Rename process._tickCallback to runNextTicks internally
    for clarity but still expose it as process._tickCallback.

Refs: #24961

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

src: move symbols binding into node_symbols.cc
62689e4
src: move process.nextTick and promise setup into node_task_queue.cc
6f95c45 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

nodejs-github-bot commented Dec 21, 2018

Copy link
Copy Markdown
Collaborator

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

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

joyeecheung commented Dec 21, 2018

Copy link
Copy Markdown
Member Author

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

joyeecheung changed the title src: move process.nextTick and promise setup into node_task_queue.cc src: set up process.nextTick and promise handlers with internalBinding('task_queue') Dec 21, 2018

joyeecheung commented Dec 22, 2018 •
edited by Trott
Loading

Copy link
Copy Markdown
Member Author

Fixed the test for --worker. CI: https://ci.nodejs.org/job/node-test-pull-request/19747/ ✔️

apapirovski approved these changes Dec 23, 2018
View reviewed changes

apapirovski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

joyeecheung commented Dec 24, 2018

Copy link
Copy Markdown
Member Author

Landed in bcea74f...457603e, thanks!

joyeecheung added a commit that referenced this pull request Dec 24, 2018
src: move symbols binding into node_symbols.cc
e830e27 
PR-URL: #25163
Refs: #24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
joyeecheung added a commit that referenced this pull request Dec 24, 2018
src: move process.nextTick and promise setup into node_task_queue.cc
457603e 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

PR-URL: #25163
Refs: #24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

MylesBorins commented Dec 25, 2018

Copy link
Copy Markdown
Contributor

This doesn't land cleanly on v11.x, should it be backported?

refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
src: move symbols binding into node_symbols.cc
700f58f 
PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
src: move process.nextTick and promise setup into node_task_queue.cc
b779b40 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
src: move symbols binding into node_symbols.cc
733af61 
PR-URL: #25163
Refs: #24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
src: move process.nextTick and promise setup into node_task_queue.cc
8abcc76 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

PR-URL: #25163
Refs: #24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

addaleax commented Jan 15, 2019

Copy link
Copy Markdown
Member

This applies cleanly now, apart from conflict resolution in parallel/test-bootstrap-modules (we should really do sth about that test…)

BridgeAR mentioned this pull request Jan 16, 2019
Merged
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
src: move symbols binding into node_symbols.cc
a9ea811 
PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
src: move process.nextTick and promise setup into node_task_queue.cc
45f906a 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
src: move symbols binding into node_symbols.cc
fa74cd3 
PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
src: move process.nextTick and promise setup into node_task_queue.cc
2e33ad1 
This patch:

- Moves the process.nextTick and promise setup C++ code into
  node_task_queue.cc which is exposed as
  `internalBinding('task_queue')`
- Makes `lib/internal/process/promises.js` and
  `lib/internal/process/next_tick.js` as side-effect-free
  as possible
- Removes the bootstrapper object being passed into
  `bootstrap/node.js`, let `next_tick.js` and `promises.js`
  load whatever they need from `internalBinding('task_queue')`
  instead.
- Rename `process._tickCallback` to `runNextTicks` internally
  for clarity but still expose it as `process._tickCallback`.

PR-URL: nodejs#25163
Refs: nodejs#24961
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins mentioned this pull request Jan 24, 2019
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

BridgeAR BridgeAR approved these changes
+1 more reviewer
apapirovski apapirovski approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL