WebNN: move release token creation into service [chromium/src : main]
Bernhart, Bryan (Gerrit)
Bernhart, Bryan added 1 comment![]( "Open in Gerrit")
Related details
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant added 1 comment![]( "Open in Gerrit")
if (release_count != 0) {IS there any validation we want to do on `release_count`? Like making sure it's monotonically increasing?
Related details
- Bernhart, Bryan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bernhart, Bryan (Gerrit)
Bernhart, Bryan added 1 comment![]( "Open in Gerrit")
if (release_count != 0) {IS there any validation we want to do on `release_count`? Like making sure it's monotonically increasing?
Anything non-zero is acceptable (looked at the `GpuChannel` contract). WebNN could strictly enforce our preferred value, but nobody else does since it isn’t required.
Related details
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant voted and added 2 comments![]( "Open in Gerrit")
Votes added by Reilly Grant
| Code-Review | +1 |
2 comments
LGTM
if (release_count != 0) {Bernhart, BryanIS there any validation we want to do on `release_count`? Like making sure it's monotonically increasing?
Anything non-zero is acceptable (looked at the `GpuChannel` contract). WebNN could strictly enforce our preferred value, but nobody else does since it isn’t required.
- Bernhart, Bryan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bernhart, Bryan (Gerrit)
Bernhart, Bryan added 1 comment![]( "Open in Gerrit")
- Chromium IPC Reviews
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: hide...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): hide...@chromium.org
Reviewer source(s):
hide...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Hidehiko Abe
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hidehiko Abe (Gerrit)
Hidehiko Abe added 1 comment![]( "Open in Gerrit")
// that will be incremented once the tensor has been exported.looks like '0' has special meaning here? Could you elaborate the details?
Related details
- Bernhart, Bryan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bernhart, Bryan (Gerrit)
Bernhart, Bryan added 1 comment![]( "Open in Gerrit")
// that will be incremented once the tensor has been exported.looks like '0' has special meaning here? Could you elaborate the details?
Correct. Updated the comment to mention this:
```
...
// A value of 0 means ExportTensor does not release a SyncToken for this
// export.
```
Related details
- Hidehiko Abe
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hidehiko Abe (Gerrit)
Hidehiko Abe added 1 comment![]( "Open in Gerrit")
// A value of 0 means ExportTensor does not release a SyncToken for this
// export.thanks for clarification.
For missing values, could you use `uint64?` and use nullopt for "missing", instead of using 0 as a special value in this case?
Related details
- Bernhart, Bryan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bernhart, Bryan (Gerrit)
Bernhart, Bryan added 1 comment![]( "Open in Gerrit")
// A value of 0 means ExportTensor does not release a SyncToken for this
// export.thanks for clarification.
For missing values, could you use `uint64?` and use nullopt for "missing", instead of using 0 as a special value in this case?
I used `uint64` to match GpuChannel.mojom and the rest of the GPU IPC stack, where a release count of 0 is already well-defined as an invalid/unreleased token. This avoids adding an extra layer of optionality. Let me know if you’d still prefer to change it.
Related details
- Hidehiko Abe
Hidehiko Abe (Gerrit)
Hidehiko Abe voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hidehiko Abe
| Code-Review | +1 |
1 comment
// A value of 0 means ExportTensor does not release a SyncToken for this
// export.Bernhart, Bryanthanks for clarification.
For missing values, could you use `uint64?` and use nullopt for "missing", instead of using 0 as a special value in this case?
I used `uint64` to match GpuChannel.mojom and the rest of the GPU IPC stack, where a release count of 0 is already well-defined as an invalid/unreleased token. This avoids adding an extra layer of optionality. Let me know if you’d still prefer to change it.
- Bernhart, Bryan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |