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: remove many uses of GetBackingStore #43923

Closed
wants to merge 2 commits into from

Conversation

kvakil
Copy link
Contributor

@kvakil kvakil commented 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: #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 <[email protected]>
    Commit-Queue: Marja Hölttä <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#81745}

Refs: v8/v8@00704f5
Refs: nodejs#32226
@nodejs-github-bot
Copy link
Collaborator

Review requested:

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

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

kvakil commented Jul 21, 2022

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

Successfully merging this pull request may close these issues.

2 participants