deps: cherry-pick 00704f5a from V8 upstream #43921

kvakil · 2022-07-21T03:04:59Z

Original commit message:

Add more efficient API for accesssing ArrayBuffer raw data

Raw data access is already possible via GetBackingStore()->GetData().
This API exposes a more efficient way for accessing
JSArrayBuffer::backing_store (which, despite the confusing name, is no
the BackingStore but its raw data pointer).

Bug: v8:10343
Change-Id: I695cea91e2c3de75ce6c86bac6e413ce6617958b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3764341
Reviewed-by: Camillo Bruni <[email protected]>
Commit-Queue: Marja Hölttä <[email protected]>
Cr-Commit-Position: refs/heads/main@{#81745}

Refs: v8/v8@00704f5
Refs: #32226

nodejs-github-bot · 2022-07-21T03:05:03Z

Review requested:

@nodejs/v8-update

This replaces many uses of `GetBackingStore()->Data()` with the newly backported `Data()`. I only replaced the "obvious" usages here, where I could easily convince myself that the lifetime was safe. The less obvious changes will come in a separate PR. Refs: nodejs#32226 Refs: nodejs#43921

cjihrig · 2022-07-21T04:37:12Z

I think this should increment v8_embedder_string in common.gypi.

This removes all usages of GetBackingStore without `CODEOWNERS`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil · 2022-07-21T04:56:08Z

I think this should increment v8_embedder_string in common.gypi.

Thanks, I missed that while skimming. I updated the docs in #43924 with a reminder.

This removes all usages of GetBackingStore without `CODEOWNERS`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

tniessen · 2022-07-21T07:31:35Z

@kvakil It looks like your git user.email is not associated with your GitHub account. This will not prevent this PR from being merged, however, we still encourage you to either re-commit the changes with an email address that is associated with your GitHub account, or that you add the email address you are using with git to your GitHub account.

kvakil · 2022-07-21T07:57:17Z

@kvakil It looks like your git user.email is not associated with your GitHub account. This will not prevent this PR from being merged, however, we still encourage you to either re-commit the changes with an email address that is associated with your GitHub account, or that you add the email address you are using with git to your GitHub account.

Thanks for the flag! I added the email to Github; it appears to have linked back correctly.

Original commit message: Add more efficient API for accesssing ArrayBuffer raw data Raw data access is already possible via GetBackingStore()->GetData(). This API exposes a more efficient way for accessing JSArrayBuffer::backing_store (which, despite the confusing name, is no the BackingStore but its raw data pointer). Bug: v8:10343 Change-Id: I695cea91e2c3de75ce6c86bac6e413ce6617958b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3764341 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Marja Hölttä <[email protected]> Cr-Commit-Position: refs/heads/main@{#81745} Refs: v8/v8@00704f5 Refs: nodejs#32226

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44076 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44075 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore without any entries in the `CODEOWNERS` file. For the most part this is a pretty straightforward review; except `SPREAD_BUFFER_ARG` and the changes to `CopyArrayBuffer`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44080 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: #32226 Refs: #43921 PR-URL: #44079 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Original commit message: Add more efficient API for accesssing ArrayBuffer raw data Raw data access is already possible via GetBackingStore()->GetData(). This API exposes a more efficient way for accessing JSArrayBuffer::backing_store (which, despite the confusing name, is no the BackingStore but its raw data pointer). Bug: v8:10343 Change-Id: I695cea91e2c3de75ce6c86bac6e413ce6617958b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3764341 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Marja Hölttä <[email protected]> Cr-Commit-Position: refs/heads/main@{#81745} Refs: v8/v8@00704f5 Refs: #32226 PR-URL: #43921 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Feng Yu <[email protected]>

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44078 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44076 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44075 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore without any entries in the `CODEOWNERS` file. For the most part this is a pretty straightforward review; except `SPREAD_BUFFER_ARG` and the changes to `CopyArrayBuffer`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44080 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: #32226 Refs: #43921 PR-URL: #44079 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Original commit message: Add more efficient API for accesssing ArrayBuffer raw data Raw data access is already possible via GetBackingStore()->GetData(). This API exposes a more efficient way for accessing JSArrayBuffer::backing_store (which, despite the confusing name, is no the BackingStore but its raw data pointer). Bug: v8:10343 Change-Id: I695cea91e2c3de75ce6c86bac6e413ce6617958b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3764341 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Marja Hölttä <[email protected]> Cr-Commit-Position: refs/heads/main@{#81745} Refs: v8/v8@00704f5 Refs: #32226 PR-URL: #43921 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Feng Yu <[email protected]>

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44078 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44076 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44075 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore without any entries in the `CODEOWNERS` file. For the most part this is a pretty straightforward review; except `SPREAD_BUFFER_ARG` and the changes to `CopyArrayBuffer`. See the linked issue for an explanation. Refs: #32226 Refs: #43921 PR-URL: #44080 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: #32226 Refs: #43921 PR-URL: #44079 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Original commit message: Add more efficient API for accesssing ArrayBuffer raw data Raw data access is already possible via GetBackingStore()->GetData(). This API exposes a more efficient way for accessing JSArrayBuffer::backing_store (which, despite the confusing name, is no the BackingStore but its raw data pointer). Bug: v8:10343 Change-Id: I695cea91e2c3de75ce6c86bac6e413ce6617958b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3764341 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Marja Hölttä <[email protected]> Cr-Commit-Position: refs/heads/main@{#81745} Refs: v8/v8@00704f5 Refs: nodejs#32226 PR-URL: nodejs#43921 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Feng Yu <[email protected]>

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44078 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44076 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44075 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore without any entries in the `CODEOWNERS` file. For the most part this is a pretty straightforward review; except `SPREAD_BUFFER_ARG` and the changes to `CopyArrayBuffer`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44080 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: nodejs#32226 Refs: nodejs#43921 PR-URL: nodejs#44079 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

nodejs-github-bot added needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Jul 21, 2022

gengjiawen approved these changes Jul 21, 2022

View reviewed changes

kvakil mentioned this pull request Jul 21, 2022

src: remove many uses of GetBackingStore #43923

Closed

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore

a2f8c27

This removes all usages of GetBackingStore without `CODEOWNERS`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore

d58b4e1

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore

afda574

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore

c07ea79

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore

5133a27

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove unowned usages of GetBackingStore

7973e4e

This removes all usages of GetBackingStore without `CODEOWNERS`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore in node-api

8e7875f

This removes all usages of GetBackingStore in `node-api`. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore in modules

5582fcf

This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore in WASI

2ec02bb

This removes all usages of GetBackingStore in WASI. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil added a commit to kvakil/node that referenced this pull request Jul 21, 2022

src: remove usages of GetBackingStore in startup

48287f2

This removes all usages of GetBackingStore in startup. See the linked issue for an explanation. Refs: nodejs#32226 Refs: nodejs#43921

kvakil mentioned this pull request Jul 21, 2022

buffer: ~2x slowdown in master compared to v12.x #32226

Open

targos approved these changes Jul 21, 2022

View reviewed changes

kvakil added 2 commits July 24, 2022 18:23

Increment embedder number.

beed835

kvakil force-pushed the backportFasterAccess branch from edffcbc to beed835 Compare July 24, 2022 18:24

F3n67u added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 25, 2022

danielleadams mentioned this pull request Aug 16, 2022

v18.8.0 release proposal #44246

Closed

ruyadorno mentioned this pull request Aug 23, 2022

v18.8.0 release proposal #44353

Merged

targos added the dont-land-on-v16.x label Sep 5, 2022

JanMarvin mentioned this pull request Nov 2, 2022

UBSAN error in GetBackingStore() jeroen/V8#152

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: cherry-pick 00704f5a from V8 upstream #43921

deps: cherry-pick 00704f5a from V8 upstream #43921

kvakil commented Jul 21, 2022

nodejs-github-bot commented Jul 21, 2022

cjihrig commented Jul 21, 2022

kvakil commented Jul 21, 2022

tniessen commented Jul 21, 2022

kvakil commented Jul 21, 2022

deps: cherry-pick 00704f5a from V8 upstream #43921

deps: cherry-pick 00704f5a from V8 upstream #43921

Conversation

kvakil commented Jul 21, 2022

nodejs-github-bot commented Jul 21, 2022

cjihrig commented Jul 21, 2022

kvakil commented Jul 21, 2022

tniessen commented Jul 21, 2022

kvakil commented Jul 21, 2022