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

timers: refactor timer callback initialization #26583

Closed
wants to merge 3 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Mar 11, 2019

timers: refactor to use module.exports

timers: reduce usage of public util

Refs: #26546

timers: refactor timer callback initialization

This patch:

  • Moves the timer callback initialization into bootstrap/node.js,
    documents when they will be called, and make the dependency on
    process._tickCallback explicit.
  • Moves the initialization of tick callbacks and timer callbacks
    to the end of the bootstrap to make sure the operations
    done before those initializations are synchronous.
  • Moves more internals into internal/timers.js from timers.js.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Mar 11, 2019
@joyeecheung joyeecheung force-pushed the refactor-timers branch 2 times, most recently from fe2f5d8 to 4a99e42 Compare March 11, 2019 11:37
@joyeecheung
Copy link
Member Author

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.

@nodejs/timers

This patch:

- Moves the timer callback initialization into bootstrap/node.js,
  documents when they will be called, and make the dependency on
  process._tickCallback explicit.
- Moves the initialization of tick callbacks and timer callbacks
  to the end of the bootstrap to make sure the operations
  done before those initializations are synchronous.
- Moves more internals into internal/timers.js from timers.js.
@joyeecheung
Copy link
Member Author

joyeecheung commented Mar 18, 2019

@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

Landed in 84156cf...1a6fb71

joyeecheung added a commit that referenced this pull request Mar 18, 2019
PR-URL: #26583
Refs: #26546
Reviewed-By: Anna Henningsen <[email protected]>
joyeecheung added a commit that referenced this pull request Mar 18, 2019
Refs: #26546

PR-URL: #26583
Reviewed-By: Anna Henningsen <[email protected]>
joyeecheung added a commit that referenced this pull request Mar 18, 2019
This patch:

- Moves the timer callback initialization into bootstrap/node.js,
  documents when they will be called, and make the dependency on
  process._tickCallback explicit.
- Moves the initialization of tick callbacks and timer callbacks
  to the end of the bootstrap to make sure the operations
  done before those initializations are synchronous.
- Moves more internals into internal/timers.js from timers.js.

PR-URL: #26583
Refs: #26546
Reviewed-By: Anna Henningsen <[email protected]>
@Fishrock123
Copy link
Contributor

I think the large comment block should have been moved as well...

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 18, 2019
To be paired with the commits from
nodejs#26583

Specifically:
1a6fb71
Fishrock123 added a commit that referenced this pull request Mar 21, 2019
To be paired with the commits from
#26583

Specifically:
1a6fb71

PR-URL: #26761
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
PR-URL: #26583
Refs: #26546
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
Refs: #26546

PR-URL: #26583
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
This patch:

- Moves the timer callback initialization into bootstrap/node.js,
  documents when they will be called, and make the dependency on
  process._tickCallback explicit.
- Moves the initialization of tick callbacks and timer callbacks
  to the end of the bootstrap to make sure the operations
  done before those initializations are synchronous.
- Moves more internals into internal/timers.js from timers.js.

PR-URL: #26583
Refs: #26546
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
To be paired with the commits from
#26583

Specifically:
1a6fb71

PR-URL: #26761
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants