SoA Proxy, main branch (2024.09.25.) #296
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krasznaa
added
enhancement
Did not try to make a fully functional vecmem::tuple_cat function, as that was not technically needed so far.
Proxies can be used to provide an "AoS view" over an SoA container. A view that would make use of the same interface that the container itself also uses. Code for device container support was also added, but is probably not functional at the moment.
While also ensuring that vecmem::edm::proxy would be usable in device code, and also with C++14.
As feared, this premature/misguided optimization was not helping with code performance in real life.
krasznaa
force-pushed
the
SoAProxy-main-20240925
branch
from
f3e7a76
to
78c80c1
Compare
|
I now tried this code with acts-project/traccc#712. @stephenswat was correct. It's more performant to just use references everywhere in the proxies, and leave it up to the GPU to figure out what it would pull into registers. As noted on acts-project/traccc#712, it turns out that memory copies with I intend to study that more deeply in some new benchmarks that I'll add in a separate PR. 🤔 |
|
After having slept on it a few times already, I'm still pretty happy with this code. So let's go forward with it. 😉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the final straw being acts-project/traccc#712, this is an attempt at providing "proxies" for the SoA containers that would allow an "AoS-like" access to the containers in the client code.
The code requires the user to create the same type of interface struct/class for their container, as they did so far. But now that interface is no longer only used providing a user-friendly API for the containers, but also to provide the same API for the proxy objects. Allowing user code to interchangeably use
container.variable().at(i)or;container.at(i).variable()semantics to access the same variable.
We discussed about this already with @stephenswat, and I'm absolutely not convinced that I did the correct thing here, but I made the proxies behave in the following way:
My idea being that this way we could force the loading of all variables of a given container in a single place in device code, when we are only reading from those variables. But as @stephenswat pointed out, I might very well be doing something silly with this. And should maybe just use references in the proxies everywhere. 🤔
The device code, and a lot of additional testing code is still to come, but I wanted to showcase already how I want to implement this feature.