FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

process: support hrtime in the snapshot by joyeecheung · Pull Request #40649 · nodejs/node · GitHub

/ node Public

process: support hrtime in the snapshot - #40649

Closed
joyeecheung wants to merge 2 commits into
nodejs:masterfrom
joyeecheung:hrtime-snapshot
Closed

process: support hrtime in the snapshot#40649
joyeecheung wants to merge 2 commits into
nodejs:masterfrom
joyeecheung:hrtime-snapshot

Conversation

joyeecheung commented Oct 29, 2021
edited
Loading

Copy link
Copy Markdown
Member

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 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

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.

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

Comment thread src/node_process.h Outdated

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member Author

cc @nodejs/startup

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}

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
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 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 mentioned this pull request Feb 1, 2022
This file contains hidden or 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
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


Back | FazBrowse Home | New Git URL