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: save the performance milestone time origin in the AliasedArray #48708

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

joyeecheung
Copy link
Member

Previously we cache the time origin for the milestones in the user land, and refresh it at pre-execution. As result the time origin gets serialized into the snapshot and is therefore not deterministic. Now we store it in the milestone array as an internal value and reset the milestones at serialization time instead of deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.

Refs: nodejs/build#3043

Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
@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. needs-ci PRs that need a full CI run. labels Jul 9, 2023
@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

Added another commit to make sure that the milestones are re-initialized again for workers. @anonrig can you take a look again? Thanks

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 20, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 20, 2023
@nodejs-github-bot nodejs-github-bot merged commit 4ee4718 into nodejs:main Jul 20, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 4ee4718

rluvaton pushed a commit to rluvaton/node that referenced this pull request Jul 21, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
pluris pushed a commit to pluris/node that referenced this pull request Aug 6, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
pluris pushed a commit to pluris/node that referenced this pull request Aug 7, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: nodejs#48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
Previously we cache the time origin for the milestones in the user
land, and refresh it at pre-execution. As result the time origin
gets serialized into the snapshot and is therefore not deterministic.
Now we store it in the milestone array as an internal value and
reset the milestones at serialization time instead of
deserialization time. This improves the determinism of the snapshot.

Drive-by: remove the unused MarkMilestone() binding.
PR-URL: #48708
Refs: nodejs/build#3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@UlisesGascon UlisesGascon mentioned this pull request Aug 15, 2023
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. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants