In addition to moving the valid check earlier in Buffer::Drop, this
patch also ensures that Device clears the tracked buffers set after they
have been unmapped, and cleans up the error handling in Device::GetLost.
Differential Revision: https://phabricator.services.mozilla.com/D191565
This ensures that both internal and external triggers of "lose the
device" resolve the lost promise using the same code path.
Differential Revision: https://phabricator.services.mozilla.com/D190129
Replace the existing callers of `JS::NewExternalArrayBuffer` with the new `UniquePtr`
alternative.
The old `JS::NewExternalArrayBuffer` function is still used in tests when the
free-function is `nullptr`.
Differential Revision: https://phabricator.services.mozilla.com/D182587
Replace the existing callers of `JS::NewExternalArrayBuffer` with the new `UniquePtr`
alternative.
The old `JS::NewExternalArrayBuffer` function is still used in tests when the
free-function is `nullptr`.
Differential Revision: https://phabricator.services.mozilla.com/D182587
Replace the existing callers of `JS::NewExternalArrayBuffer` with the new `UniquePtr`
alternative.
The old `JS::NewExternalArrayBuffer` function is still used in tests when the
free-function is `nullptr`.
Depends on D182586
Differential Revision: https://phabricator.services.mozilla.com/D182587
As of the prior patch, these are no longer needed. I removed
these with a script, then ran clang-format on the files, then
manually reverted a few unrelated changed from the formatter.
Differential Revision: https://phabricator.services.mozilla.com/D164829
The former frees resources but keeps the handle. It can be called multiple times. The latter destroys the handle. Any subsequent reference to the same buffer is a bug and will cause the GPU process to crash.
Depends on D152080
Differential Revision: https://phabricator.services.mozilla.com/D152081
Having the code in the same place makes it easier to follow. This made me realize that the validation of aMode in mapAsync has to move to the device side (fix coming in a followup).
Depends on D151631
Differential Revision: https://phabricator.services.mozilla.com/D151632
Another cosmetic change. I've dabbled with IPDL actors too much to not think about WebGPUParent when reading "mParent". Also the parent-child relationship between Device and Buffer is not very obvious to me (nor is it part of the specification).
So I find that wrapping mParent in a GetDevice method to make the code easier to understand. It also makes it explicit that the parent pointer cannot be null.
Depends on D151630
Differential Revision: https://phabricator.services.mozilla.com/D151631
Make sure to always clean up any potential content-side state and only avoid sending Destroy each time.
Depends on D151621
Differential Revision: https://phabricator.services.mozilla.com/D151629
Per spec (and discussion with someone on the chromium side where spec is vague), the correct behavior should be:
- MapAsync validation happens on the device timeline, so we should reject the promise in mapAsync on the content side if we run into an internal error not described by the spec.
- Unmap immediately rejects all pending mapping promises on the content side (there can be multiple of them since we have to catch that error on the device timeline).
This patch tracks a single mapping promise at a time and immediately rejects on the content side any subseqent mapping
request made until unmap is called. This means our current implementation deviates slightly from the current state of
the spec in that:
- The promise is rejected earlier on the content timeline,
- If the first request fails, all subsequent requests will fail until either unmap or when the content side receives and processes the rejected
promise, whereas Dawn's implementation would allow the first valid request to succed.
There was some confusion around the the use of uint64_t and size_t which probably originated at point where this code was working differently. This patch uses uint64_t (=BufferAddress) more consistently removing the need for some of the casting and overflow checks.
One notable change in the overall logic is that SetMapped is now called when the buffer is actually in the mapped state (before this patch it was called as soon as the buffer had a pending map request).
Depends on D151618
Differential Revision: https://phabricator.services.mozilla.com/D151619
No funcitonal change here, I like to have the code maintaining and depending on the same invariants to be in the same place.
Depends on D151616
Differential Revision: https://phabricator.services.mozilla.com/D151617
This commit makes WebGPU buffers use unsafe shmems.
Instead of relying on moving the shmem back and forth between the two processes to ensure thread safety, we instead rely on the validation done on both sides. The upside is that it makes it much easier to implement said validation correctly.
In the interest of splitting the buffer mapping rework into small-ish commits, this one puts some structural pieces in place but doesn't necessarily do justice to the final implementation:
- The validation itself is coming in subsequent patches in this series.
- Mapping sub-ranges of the buffer was somewhat implemented in some parts of the parent code and not in others, and was fairly broken as a whole. This commit always maps the entire buffer and proper logic for sub-ranges is coming in another commit.
The main things this commit does put in place:
- Each mappable buffer is associated with a Shmem that is accessible to both sides.
- On the child side, if a buffer is not mappable, then Buffer::mShmem is in its default state (it doesn't point to any shared memory segment).
- On the parent side, if a buffer is not mappable it does not have an entry in mSharedMemoryMap.
- The shmem is always created by the child and destroyed by the parent.
Depends on D151615
Differential Revision: https://phabricator.services.mozilla.com/D151616
Most operations maniplating shmems in WebGPU are fallible, we'll have to handle passing them conditionally in most messages.
This commit starts with BufferMap, to avoid crashing when map is called on an invalid buffer.
Differential Revision: https://phabricator.services.mozilla.com/D149892
The former frees resources but keeps the handle. It can be called multiple times. The latter destroys the handle. Any subsequent reference to the same buffer is a bug and will cause the GPU process to crash.
Depends on D152080
Differential Revision: https://phabricator.services.mozilla.com/D152081
Having the code in the same place makes it easier to follow. This made me realize that the validation of aMode in mapAsync has to move to the device side (fix coming in a followup).
Depends on D151631
Differential Revision: https://phabricator.services.mozilla.com/D151632
Another cosmetic change. I've dabbled with IPDL actors too much to not think about WebGPUParent when reading "mParent". Also the parent-child relationship between Device and Buffer is not very obvious to me (nor is it part of the specification).
So I find that wrapping mParent in a GetDevice method to make the code easier to understand. It also makes it explicit that the parent pointer cannot be null.
Depends on D151630
Differential Revision: https://phabricator.services.mozilla.com/D151631
Make sure to always clean up any potential content-side state and only avoid sending Destroy each time.
Depends on D151621
Differential Revision: https://phabricator.services.mozilla.com/D151629
Per spec (and discussion with someone on the chromium side where spec is vague), the correct behavior should be:
- MapAsync validation happens on the device timeline, so we should reject the promise in mapAsync on the content side if we run into an internal error not described by the spec.
- Unmap immediately rejects all pending mapping promises on the content side (there can be multiple of them since we have to catch that error on the device timeline).
This patch tracks a single mapping promise at a time and immediately rejects on the content side any subseqent mapping
request made until unmap is called. This means our current implementation deviates slightly from the current state of
the spec in that:
- The promise is rejected earlier on the content timeline,
- If the first request fails, all subsequent requests will fail until either unmap or when the content side receives and processes the rejected
promise, whereas Dawn's implementation would allow the first valid request to succed.
There was some confusion around the the use of uint64_t and size_t which probably originated at point where this code was working differently. This patch uses uint64_t (=BufferAddress) more consistently removing the need for some of the casting and overflow checks.
One notable change in the overall logic is that SetMapped is now called when the buffer is actually in the mapped state (before this patch it was called as soon as the buffer had a pending map request).
Depends on D151618
Differential Revision: https://phabricator.services.mozilla.com/D151619
No funcitonal change here, I like to have the code maintaining and depending on the same invariants to be in the same place.
Depends on D151616
Differential Revision: https://phabricator.services.mozilla.com/D151617
This commit makes WebGPU buffers use unsafe shmems.
Instead of relying on moving the shmem back and forth between the two processes to ensure thread safety, we instead rely on the validation done on both sides. The upside is that it makes it much easier to implement said validation correctly.
In the interest of splitting the buffer mapping rework into small-ish commits, this one puts some structural pieces in place but doesn't necessarily do justice to the final implementation:
- The validation itself is coming in subsequent patches in this series.
- Mapping sub-ranges of the buffer was somewhat implemented in some parts of the parent code and not in others, and was fairly broken as a whole. This commit always maps the entire buffer and proper logic for sub-ranges is coming in another commit.
The main things this commit does put in place:
- Each mappable buffer is associated with a Shmem that is accessible to both sides.
- On the child side, if a buffer is not mappable, then Buffer::mShmem is in its default state (it doesn't point to any shared memory segment).
- On the parent side, if a buffer is not mappable it does not have an entry in mSharedMemoryMap.
- The shmem is always created by the child and destroyed by the parent.
Depends on D151615
Differential Revision: https://phabricator.services.mozilla.com/D151616
Most operations maniplating shmems in WebGPU are fallible, we'll have to handle passing them conditionally in most messages.
This commit starts with BufferMap, to avoid crashing when map is called on an invalid buffer.
Differential Revision: https://phabricator.services.mozilla.com/D149892
Brad Werth
Nicolas Silva
Erich Gubler
Narcis Beleuzu
André Bargull
Sandor Molnar
Cristina Horotan
Andrew McCreight
Cristian Tuns
Marian-Vasile Laza