acts-project / vecmem

Vectorised data model base and helper classes.
https://acts-project.github.io/vecmem/
Mozilla Public License 2.0
19 stars 13 forks source link

Add ability to reserve bulk space in device vector #274

Closed stephenswat closed 5 months ago

stephenswat commented 7 months ago

A very common pattern in massively parallel computing is to reserve space for an entire thread block at once and to insert those values later. This commit adds a reserve and reserve_unsafe method to device_vector, which allows clients to do this.

The semantics of reserve are fully compliant with the C++ standard starting from C++20; C++ standards below that will work just fine. Any program which calls this function and compiles should be well formed, as defined in P0593R6 and P2590R2.

The reserve_unsafe method does the same, but technically leaves the program in an ill-defined state. Note that this form of ill-defined memory has been used for donkeys years and is actually safe. It's just technically not standards compliant.

stephenswat commented 7 months ago

Agreed the naming could be improved; I see a few functions that we could implement which would need a name:

  1. Implicitly insert $n$ elements, not calling their constructors for performance reasons, but the objects are well-formed because they are implicit lifetime objects.
  2. Implicitly insert $n$ elements, not calling their constructors for performance reasons, in an unsafe way because they are not implicit lifetime objects.
  3. Insert $n$ elements, default-constructing each of them.
  4. Insert $n$ elements, copy-constructing them from a given value.

The first two are implemented, I agree the second two could also be useful; but they need names.

stephenswat commented 7 months ago

Could be something like bulk_insert_implicit, bulk_insert_implicit_unsafe, bulk_insert (with 2 overloads)?

krasznaa commented 7 months ago

As long as we're workshopping the names, I'd prefer bulk_append_implicit, etc. Since these will only ever add new elements at the end of the vector.

stephenswat commented 7 months ago

I have renamed the functions and added the additional two functions I described above.

stephenswat commented 5 months ago

I think this should be good to go now. I'd suggest we get this and #275 in and tag a release.

krasznaa commented 5 months ago

Are you still tweaking it, or is it ready? 😕

stephenswat commented 5 months ago

This is now ready to go. :+1:

krasznaa commented 5 months ago

Good. Then let me just put some unit tests on top of it, which I invite you to check. 😉

stephenswat commented 5 months ago

Looks sane. :+1:

stephenswat commented 5 months ago

Let's finally get this in. :smile: