Skip to content

Conversation

@joyeecheung
Copy link
Member

@joyeecheungjoyeecheung 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-botnodejs-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
MemberAuthor

cc @nodejs/startup

@aduh95aduh95 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 <[email protected]>
@joyeecheung
Copy link
MemberAuthor

Landed in 9aeb836

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 <[email protected]>
This was referenced Feb 16, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs 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 / srcIssues 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

@joyeecheung@nodejs-github-bot@jasnell@targos@aduh95