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

process: support hrtime in the snapshot #40649

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 29, 2021

Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into Environment::SetFastMethod() for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.

This is split from #38905

Refs: #35711
Refs: #37476

@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 Oct 29, 2021
@nodejs-github-bot

This comment has been minimized.

Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into `Environment::SetFastMethod()` for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

src/node_process.h Outdated Show resolved Hide resolved
src/node_process_methods.cc Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

cc @nodejs/startup

@joyeecheung joyeecheung added the review wanted PRs that need reviews. label Nov 5, 2021
targos pushed a commit to targos/abi-stable-v8 that referenced this pull request Nov 9, 2021
The tests are modeled after another patch that includes
v8::CFunctions into Node.js's builtin snapshot.

Refs: nodejs/node#40649
Change-Id: I5a91682f7944ef06a0d3caf7333b09f974bcd64b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3251138
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/main@{#77726}
targos pushed a commit to targos/abi-stable-v8 that referenced this pull request Nov 9, 2021
This reverts commit 5dd16ca.

Reason for revert: MSAN complains about an uninitialized value, see https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/41150/overview

Original change's description:
> Add tests for serialization of v8::CFunction
>
> The tests are modeled after another patch that includes
> v8::CFunctions into Node.js's builtin snapshot.
>
> Refs: nodejs/node#40649
> Change-Id: I5a91682f7944ef06a0d3caf7333b09f974bcd64b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3251138
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Maya Lekova <mslekova@chromium.org>
> Commit-Queue: Joyee Cheung <joyee@igalia.com>
> Cr-Commit-Position: refs/heads/main@{#77726}

Change-Id: I9ea32a84783c3f555ee40daebf7b7f6c74f75062
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3263892
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Owners-Override: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#77729}
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

CI was green. Landed in e6d8ae0

joyeecheung added a commit that referenced this pull request Nov 17, 2021
Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into `Environment::SetFastMethod()` for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.

PR-URL: #40649
Refs: #35711
Refs: #37476
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung joyeecheung removed review wanted PRs that need reviews. needs-ci PRs that need a full CI run. labels Nov 19, 2021
targos pushed a commit that referenced this pull request Nov 21, 2021
Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into `Environment::SetFastMethod()` for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.

PR-URL: #40649
Refs: #35711
Refs: #37476
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into `Environment::SetFastMethod()` for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.

PR-URL: #40649
Refs: #35711
Refs: #37476
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
Put the hrtime backing store in the process methods binding data
so that it can be integrated into the snapshot builder. For
now we simply discard the contents of the hrtime buffer during
serialization and create new buffers upon deserialization because
the contents are only useful in a synchronous call.

This also moves the helper function for creating V8 FastAPI methods
into `Environment::SetFastMethod()` for code reuse. The v8::CFunction
need to be created before the Environment is created so that we
have the CTypeInfo address available for external reference
registration.

PR-URL: #40649
Refs: #35711
Refs: #37476
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants