-
Notifications
You must be signed in to change notification settings - Fork 30k
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
bootstrap: include v8 and fs modules in the startup snapshot #36943
Closed
Conversation
This file contains 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
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 15, 2021
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
January 15, 2021 14:36
831d67d
to
75283c6
Compare
CI is failing because the fs modules aren't yet vetted for runtime-dependence (so e.g. |
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
January 28, 2021 12:22
75283c6
to
e9d9076
Compare
This was referenced Jan 28, 2021
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
January 28, 2021 16:33
e9d9076
to
357cd1d
Compare
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
January 28, 2021 16:41
357cd1d
to
451c7a7
Compare
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
added a commit
that referenced
this pull request
Feb 8, 2021
Previously, this was a per-class string constant for BindingData which is used as keys for identifying these objects in the binding data map. These are just type names of the BindingData. This patch renames the variable to type_name so that we can generalize this constant for other BaseObjects and use it for debugging and logging the types of other BaseObjects. PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
joyeecheung
added a commit
that referenced
this pull request
Feb 8, 2021
1. Put the v8 binding data class into a header so we can reuse the class definition during deserialization. 2. Put the v8 binding code into node::v8_utils namespace for clarity. 3. Move the binding data property initialization into its constructor so that we can reuse it during deserialization 4. Reorder the v8 binding initialization so that we don't unnecessarily initialize the properties in a loop PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
February 8, 2021 16:33
451c7a7
to
bb64b7d
Compare
joyeecheung
changed the title
WIP: support v8 and fs BindingData in the startup snapshot
bootstrap: support v8 and fs BindingData in the startup snapshot
Feb 9, 2021
joyeecheung
changed the title
bootstrap: support v8 and fs BindingData in the startup snapshot
bootstrap: include v8 and fs modules in the startup snapshot
Feb 9, 2021
cc @nodejs/startup This is now ready for review |
jasnell
approved these changes
Feb 14, 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>
danielleadams
pushed a commit
that referenced
this pull request
Feb 16, 2021
Previously, this was a per-class string constant for BindingData which is used as keys for identifying these objects in the binding data map. These are just type names of the BindingData. This patch renames the variable to type_name so that we can generalize this constant for other BaseObjects and use it for debugging and logging the types of other BaseObjects. PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
danielleadams
pushed a commit
that referenced
this pull request
Feb 16, 2021
1. Put the v8 binding data class into a header so we can reuse the class definition during deserialization. 2. Put the v8 binding code into node::v8_utils namespace for clarity. 3. Move the binding data property initialization into its constructor so that we can reuse it during deserialization 4. Reorder the v8 binding initialization so that we don't unnecessarily initialize the properties in a loop PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
This patch adds the SnapshotableObject interface. Native objects supporting serialization can inherit from it, implementing PrepareForSerialization(), Serialize() and Deserialize() to control how the native states should be serialized and deserialized. See doc: https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit
joyeecheung
force-pushed
the
baseobj-reinit
branch
from
February 19, 2021 09:23
bb64b7d
to
8da45ff
Compare
joyeecheung
added a commit
that referenced
this pull request
Feb 19, 2021
This patch adds the SnapshotableObject interface. Native objects supporting serialization can inherit from it, implementing PrepareForSerialization(), Serialize() and Deserialize() to control how the native states should be serialized and deserialized. See doc: https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit PR-URL: #36943 Fixes: #35930 Refs: #35711 Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 05286b9...445108d |
It seems this PR broke the ASAN CI:
GH Actions is failing consistently since this landed on master. |
targos
pushed a commit
that referenced
this pull request
Feb 28, 2021
This patch adds the SnapshotableObject interface. Native objects supporting serialization can inherit from it, implementing PrepareForSerialization(), Serialize() and Deserialize() to control how the native states should be serialized and deserialized. See doc: https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit PR-URL: #36943 Fixes: #35930 Refs: #35711 Reviewed-By: James M Snell <jasnell@gmail.com>
targos
pushed a commit
that referenced
this pull request
May 1, 2021
Previously, this was a per-class string constant for BindingData which is used as keys for identifying these objects in the binding data map. These are just type names of the BindingData. This patch renames the variable to type_name so that we can generalize this constant for other BaseObjects and use it for debugging and logging the types of other BaseObjects. PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
targos
pushed a commit
that referenced
this pull request
May 1, 2021
1. Put the v8 binding data class into a header so we can reuse the class definition during deserialization. 2. Put the v8 binding code into node::v8_utils namespace for clarity. 3. Move the binding data property initialization into its constructor so that we can reuse it during deserialization 4. Reorder the v8 binding initialization so that we don't unnecessarily initialize the properties in a loop PR-URL: #37112 Refs: #36943 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #35930
Refs: #35711
The basic idea is:
src: support serialization of binding data
This patch adds the SnapshotableObject interface. Native objects
supporting serialization can inherit from it, implementing
PrepareForSerialization(), Serialize() and Deserialize()
to control how the native states should be serialized and
deserialized.
See doc: https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit
bootstrap: include fs module into the builtin snapshot
bootstrap: include v8 module into the builtin snapshot