Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let Player figure out the backend part of IDs #792

Closed
kvark opened this issue Jul 13, 2020 · 2 comments
Closed

Let Player figure out the backend part of IDs #792

kvark opened this issue Jul 13, 2020 · 2 comments
Labels
help required We need community help to make this happen. type: enhancement New feature or request

Comments

@kvark
Copy link
Member

kvark commented Jul 13, 2020

Is your feature request related to a problem? Please describe.
When API traces are serialized, we are saving each ID as a triple of (index, epoch, backend), e.g.:

id: Id(0, 1, Metal)

This is verbose and redundant, also incorrect from the player point of view. The player picks the backend independently and just wants to treat all the IDs as belonging to this backend. So we don't want to have the backend specified in a trace, other than maybe once at the top for informational reasons only.

Describe the solution you'd like
@lachlansneff suggested to have a custom serializer/deserializer implementation for traces. It would behave just like the standard one from serde, but omit the backend on serialization and recover it on deserialization, based on a global setting.
Ideally, it would look something like:

Id(index: 0, epoch: 1)

Describe alternatives you've considered
A text-based search&replace would also work, but it's a bit awkward.
Besides, we don't want to pollute traces with irrelevant data.

Additional context
We could live without this for the matter of replaying traces manually, but it now it's needed for #786

@kvark kvark added the type: enhancement New feature or request label Jul 13, 2020
@kvark
Copy link
Member Author

kvark commented Jul 14, 2020

I tries to see where this would be going. A blanket Serializer implementation is 150 LOC... This is quite unfortunate.
I wonder if there is a way to sorta hack Rust trait system in a way that we can have a wrapper around a trait implementation, where we only override a few methods, leaving other ones untouched.

serializer.txt

bors bot added a commit that referenced this issue Jul 17, 2020
803: Player-based GPU test framework r=cwfitzgerald a=kvark

**Connections**
Closes #786

**Description**
This change adds a GPU-based testing by re-using the Player from tracing infrastructure - #289.
It converts the player into a lib + binary, and adds an integration test into the crate that implements RON-specified testing.

Current implementation has a few requirements/gotchas that are listed in `test.rs`:
* in all the IDs, the backend is `Empty`
* all expected buffers have `MAP_READ` usage on them
* last action is `Submit`

I believe it's workable, and we can improve it down the road (e.g. with #792).

**Testing**
MUHAHAHA
`cargo test` nails it

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
kvark added a commit to kvark/wgpu that referenced this issue Jun 3, 2021
792: Use implicit layout for hello-compute r=kvark a=kvark



Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@cwfitzgerald cwfitzgerald added the help required We need community help to make this happen. label Jun 5, 2022
@teoxoy
Copy link
Member

teoxoy commented Dec 11, 2024

No longer relevant; we removed the backend in IDs in #6263.

@teoxoy teoxoy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help required We need community help to make this happen. type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants