Sitelet https://github.com/gpuweb/gpuweb/pull/2837
Skip to content

buffer/device.destroy() unmaps buffer on current thread - #2837

Merged
kainino0x merged 2 commits into
gpuweb:mainfrom
kainino0x:destroy-unmaps
May 16, 2022
Merged

kainino0x merged 2 commits into
gpuweb:mainfrom
kainino0x:destroy-unmaps

Conversation

@kainino0x

@kainino0x kainino0x commented May 6, 2022 •

Copy link
Copy Markdown
Contributor

Adds some language that assumes multiple threads, which we don't have,
but I think it's fine. Better to have this codified now.

Unfortunately the language is hand-wavy, but this isn't going to get
fixed until we have a clean client-vs-shared-vs-server state split.

Fixes #2545


Preview | Diff

Adds some language that assumes multiple threads, which we don't have,
but I think it's fine. Better to have this codified now.

Unfortunately the language is hand-wavy, but this isn't going to get
fixed until we have a clean client-vs-shared-vs-server state split.

Fixes gpuweb#2545
@kainino0x kainino0x added for webgpu editors meeting tacit resolution candidate Editors may be able to resolve and move to tacit resolution queue labels May 6, 2022
@kainino0x kainino0x added this to the V1.0 milestone May 6, 2022
@kainino0x
kainino0x requested a review from toji May 6, 2022 22:03

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TIL that browser's refer to their "threads" as agents! 🧑‍🎓

LGTM.

@kainino0x kainino0x added tacit resolution queue Editors have agreed and intend to land if no feedback is given and removed for webgpu editors meeting tacit resolution candidate Editors may be able to resolve and move to tacit resolution queue labels May 6, 2022
@kainino0x

kainino0x commented May 11, 2022 •

Copy link
Copy Markdown
Contributor Author

meeting: No objection (yet) to having destroy() do unmaps.

I'll remove the references to threading and leave them for later when we spec multithreading. What's important is just that we know there's a possible solution for later.

@kainino0x kainino0x self-assigned this May 11, 2022
@kainino0x

Copy link
Copy Markdown
Contributor Author

I just commented it out because I don't want to lose track of proposed solutions for multithreading.

@kainino0x
kainino0x requested a review from toji May 11, 2022 22:42

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with threading text commented out.

@kainino0x
kainino0x merged commit d71ec16 into gpuweb:main May 16, 2022
@kainino0x
kainino0x deleted the destroy-unmaps branch May 16, 2022 22:05
@kainino0x kainino0x removed the tacit resolution queue Editors have agreed and intend to land if no feedback is given label May 16, 2022
github-actions Bot added a commit that referenced this pull request May 16, 2022
SHA: d71ec16
Reason: push, by @kainino0x

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request May 16, 2022
SHA: d71ec16
Reason: push, by @kainino0x

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request May 16, 2022
SHA: d71ec16
Reason: push, by @kainino0x

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@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 Jun 29, 2022
@kainino0x kainino0x removed the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Jul 20, 2022
jdarpinian pushed a commit to jdarpinian/gpuweb that referenced this pull request Aug 12, 2022
* buffer/device.destroy() unmaps buffer on current thread

Adds some language that assumes multiple threads, which we don't have,
but I think it's fine. Better to have this codified now.

Unfortunately the language is hand-wavy, but this isn't going to get
fixed until we have a clean client-vs-shared-vs-server state split.

Fixes gpuweb#2545

* Comment out stuff about multithreading
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.

Should GPUDevice.destroy detach all buffer mapping ArrayBuffers?

2 participants