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

src: remove many uses of GetBackingStore by kvakil · Pull Request #43923 · nodejs/node · GitHub

/ node Public

src: remove many uses of GetBackingStore - #43923

Closed
kvakil wants to merge 2 commits into
nodejs:mainfrom
kvakil:obvious-backingstore-removals
Closed

src: remove many uses of GetBackingStore#43923
kvakil wants to merge 2 commits into
nodejs:mainfrom
kvakil:obvious-backingstore-removals

Conversation

kvakil commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

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: #32226
Refs: #43921

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 <cbruni@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#81745}

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

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules
  • @nodejs/node-api
  • @nodejs/startup
  • @nodejs/v8-update
  • @nodejs/vm
  • @nodejs/wasi

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 21, 2022
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
kvakil force-pushed the obvious-backingstore-removals branch from 554fdf2 to 261134e Compare July 21, 2022 04:22

kvakil commented Jul 21, 2022

Copy link
Copy Markdown
Contributor Author

I'm sorry, I did not realize this would cc six groups when marked as draft. :\

I am assuming six or so separate PRs would be easier to review and I'll close this while I split it up...

kvakil closed this Jul 21, 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. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL