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

src: refactor bookkeeping of bootstrap status #37113

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Jan 28, 2021

This patch

  1. Refactors the bootstrap routine of the main instance so that
    when --no-node-snapshot is used,
    Environment::InitializeMainContext() will only be called once
    (previously it would be called twice, which was harmless for now
    but not ideal).
  2. Mark the number of BaseObjects in RunBootstrapping() when creating
    the Environment from scratch and in InitializeMainContext() when
    the Environment is deserialized. Previously the marking was done in
    the Environment constructor and InitializeMainContext() respectively
    for the cctest which was incorrect because the cctest never uses
    an Environment that's not bootstrapped. Also renames the mark
    to base_object_created_after_bootstrap to reflect what it's
    intended for.

Refs: #36943
Refs: #35711

This patch

1. Refactors the bootstrap routine of the main instance so that
  when --no-node-snapshot is used,
  Environment::InitializeMainContext() will only be called once
  (previously it would be called twice, which was harmless for now
  but not ideal).
2. Mark the number of BaseObjects in RunBootstrapping() when creating
  the Environment from scratch and in InitializeMainContext() when
  the Environment is deserialized. Previously the marking was done in
  the Environment constructor and InitializeMainContext() respectively
  for the cctest which was incorrect because the cctest never uses
  an Environment that's not bootstrapped. Also renames the mark
  to base_object_created_after_bootstrap to reflect what it's
  intended for.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jan 28, 2021
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

cc @nodejs/startup

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 5, 2021
joyeecheung added a commit that referenced this pull request Feb 5, 2021
This patch

1. Refactors the bootstrap routine of the main instance so that
  when --no-node-snapshot is used,
  Environment::InitializeMainContext() will only be called once
  (previously it would be called twice, which was harmless for now
  but not ideal).
2. Mark the number of BaseObjects in RunBootstrapping() when creating
  the Environment from scratch and in InitializeMainContext() when
  the Environment is deserialized. Previously the marking was done in
  the Environment constructor and InitializeMainContext() respectively
  for the cctest which was incorrect because the cctest never uses
  an Environment that's not bootstrapped. Also renames the mark
  to base_object_created_after_bootstrap to reflect what it's
  intended for.

PR-URL: #37113
Refs: #36943
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung
Copy link
Member Author

Landed in 9aeb836

@joyeecheung joyeecheung closed this Feb 5, 2021
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
This patch

1. Refactors the bootstrap routine of the main instance so that
  when --no-node-snapshot is used,
  Environment::InitializeMainContext() will only be called once
  (previously it would be called twice, which was harmless for now
  but not ideal).
2. Mark the number of BaseObjects in RunBootstrapping() when creating
  the Environment from scratch and in InitializeMainContext() when
  the Environment is deserialized. Previously the marking was done in
  the Environment constructor and InitializeMainContext() respectively
  for the cctest which was incorrect because the cctest never uses
  an Environment that's not bootstrapped. Also renames the mark
  to base_object_created_after_bootstrap to reflect what it's
  intended for.

PR-URL: #37113
Refs: #36943
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. 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.

5 participants