Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2797653002:
[wasm] Module Builder v8 API: bytes passed in are owned by caller. (Closed)
|
Created:
3 years, 8 months ago by Mircea Trofin Modified:
3 years, 8 months ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
Description[wasm] Module Builder v8 API: bytes passed in are owned by caller.
This reflects both the contract in blink, as well as what we
plan to do in streamed compilation, where we'll want to lay out
bytes received such that each section and each function body is
contiguous, but they may all be separate - which entails a copy.
BUG=chromium:697028
Review-Url: https://codereview.chromium.org/2797653002
Cr-Commit-Position: refs/heads/master@{#44387}
Committed: https://chromium.googlesource.com/v8/v8/+/0bd9f1b8e60466fcfbbb01b5d048c72e3d01d361
Patch Set 1 #Patch Set 2 : . #
Messages
Total messages: 29 (24 generated)
The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [wasm] Module Builder v8 API: bytes passed in are owned by caller. This reflects both the contract in blink, as well as what we plan to do in streamed compilation, where we'll want to lay out bytes received such that each section and each function body is contiguous, but they may all be separate - which entails a copy. BUG= ========== to ========== [wasm] Module Builder v8 API: bytes passed in are owned by caller. This reflects both the contract in blink, as well as what we plan to do in streamed compilation, where we'll want to lay out bytes received such that each section and each function body is contiguous, but they may all be separate - which entails a copy. BUG=chromium:697028 ==========
mtrofin@chromium.org changed reviewers: + ahaas@chromium.org, bradnelson@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
This came out as part of feedback for the blink side (https://codereview.chromium.org/2797653002/). It also aligns with our plans.
The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
LGTM once the comment is addressed. https://codereview.chromium.org/2797653002/diff/20001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2797653002/diff/20001/include/v8.h#newcode4057 include/v8.h:4057: void OnBytesReceived(const std::unique_ptr<const uint8_t[]>&, size_t size); With the use of std::unique_ptr you indicate that ownership goes over to the callee. If you don't want to transfer ownership, just pass in the byte array as uint8_t*.
The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2797653002/diff/20001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2797653002/diff/20001/include/v8.h#newcode4057 include/v8.h:4057: void OnBytesReceived(const std::unique_ptr<const uint8_t[]>&, size_t size); On 2017/04/04 07:51:08, ahaas wrote: > With the use of std::unique_ptr you indicate that ownership goes over to the > callee. If you don't want to transfer ownership, just pass in the byte array as > uint8_t*. Good point - thanks!
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mtrofin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org Link to the patchset: https://codereview.chromium.org/2797653002/#ps60001 (title: ".")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491322864828180,
"parent_rev": "6d89de7b9e5fc3a5f9d0fa2cef51864191d5ff5f", "commit_rev":
"0bd9f1b8e60466fcfbbb01b5d048c72e3d01d361"}
Message was sent while issue was closed.
Description was changed from ========== [wasm] Module Builder v8 API: bytes passed in are owned by caller. This reflects both the contract in blink, as well as what we plan to do in streamed compilation, where we'll want to lay out bytes received such that each section and each function body is contiguous, but they may all be separate - which entails a copy. BUG=chromium:697028 ========== to ========== [wasm] Module Builder v8 API: bytes passed in are owned by caller. This reflects both the contract in blink, as well as what we plan to do in streamed compilation, where we'll want to lay out bytes received such that each section and each function body is contiguous, but they may all be separate - which entails a copy. BUG=chromium:697028 Review-Url: https://codereview.chromium.org/2797653002 Cr-Commit-Position: refs/heads/master@{#44387} Committed: https://chromium.googlesource.com/v8/v8/+/0bd9f1b8e60466fcfbbb01b5d048c72e3d0... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/v8/v8/+/0bd9f1b8e60466fcfbbb01b5d048c72e3d0... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
