Skip to content

Conversation

@kainino0x
Copy link
Contributor

Issue: #1629

@kainino0x kainino0x added the tacit resolution queue Editors have agreed and intend to land if no feedback is given label Mar 8, 2023
@kainino0x kainino0x added this to the V1.0 milestone Mar 8, 2023
@kainino0x kainino0x requested a review from toji March 8, 2023 02:34
information and should never be parsed by applications.
</div>
1. Complete any outstanding {{GPUBuffer/mapAsync()}} steps.
1. Complete any outstanding {{GPUQueue/onSubmittedWorkDone()}} steps.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit handwavy but I think it's OK. There are cross-references in both directions between here and those steps.

1. Return.
1. [=Assert=] |p| is still pending.
1. Set |this|.{{GPUBuffer/[[pending_map]]}} to `null`,
and [=reject=] |p| with an {{OperationError}}.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means we use an OperationError in the case of the device being lost. I think this is slightly better than AbortError because right now AbortError only happens with explicit cancellation in unmap().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. You could make a case for AbortError if the device loss was caused by an explicit destroy but I don't think anyone wants to track it that closely, nor do I think it matters to developers at that point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

device.destroy() will actually call buffer.unmap() which is supposed to cause an AbortError (if the map and the device.destroy were on the same thread). Something to test, I'll go ahead and file this so I can mention that.

@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Mar 15, 2023
@kdashg
Copy link
Contributor

kdashg commented Mar 15, 2023

GPU Web 2023-03-08
  • Specify behavior of onSubmittedWorkDone/mapAsync on device loss #3937
    • KN: proposal: onSubmittedWorkDone resolves successfully. After device loss, keeps pretending it's working. mapAsync rejects the promise though. Might change in the future, but for V1, best to reject.

Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good refactoring of the steps. LGTM!

1. Return.
1. [=Assert=] |p| is still pending.
1. Set |this|.{{GPUBuffer/[[pending_map]]}} to `null`,
and [=reject=] |p| with an {{OperationError}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. You could make a case for AbortError if the device loss was caused by an explicit destroy but I don't think anyone wants to track it that closely, nor do I think it matters to developers at that point.

@kainino0x kainino0x merged commit d958968 into gpuweb:main Mar 17, 2023
@kainino0x kainino0x deleted the async-on-lost branch March 17, 2023 01:24
@kainino0x kainino0x removed tacit resolution queue Editors have agreed and intend to land if no feedback is given needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet labels Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants