Put/Span
Created by: germasch
So I'm kinda reluctant to open yet another issue related to spans, nor do I actually much feel like going through this discussion again. Then again, it's still an open issue, so I'll try to summarize where I think we're at.
-
Put(Sync)
andPut(Deferred) -> PerformPuts()
mirrorMPI_Send
andMPI_Isend -> MPI_Wait
in terms of when data is ready when data can be touched by whom.
If this is not what ADIOS2's API meant to express, what follows may not apply.
- A way for the app to write directly into ADIOS2's internal buffer can be useful in certain cases, because it avoids a copy of the data (avoids performance penalty) and allocating a second data buffer in the first place (avoids allocating twice as much memory as actually needed.) (#1315)
I think we agree on that. The current Span interface achieves those goals. It comes with caveats (I'm trying to summarize #1290 primarily -- that's a long thread, so the below won't be complete. If you feel I forgot anything important, let me know and I'll add it):
- The memory it returns may not be aligned to usual malloc guarantees (#1314 (closed)). This may or may not be fixable, and it may or may no be an actual problem for users of the Span interface (but it should be documented at least)
- If the API to return the buffer to be filled to the user is called
Put
, that breaks the expectation that data is ready at the time ofPut
, which is currently satisfied by the two existingPut(Sync)
andPut(Deferred)
. - The buffer returned to the user is subject to change if the underlying adios2 buffer gets realloced. This should be documented. To me, it also means that the choice "Span" is a questionable name for what's returned, because it was meant to imply an analogy to
std::span
, butstd::span
never changesdata()
under you. Anyway, can be handled with documentation. - The current implementation of this interface uses
Put
to return the buffer, but then doesn't usePut
at the time the data is ready, so implicitly, the data is ready atPerformPuts
time. To me, that's not consistent with the existing interface, and also means that there is no API to support potential overlap (the Put(Deferred) -> PerformPuts type) for engines that can use it, like InsituMPI.
So what I suggested in #1290 are two changes:
- rename the Span-returning function to
PutPrealloc
(The actual name is a suggestion that could be discussed (one alternative would beGetPutBuffer
or something like this). I don't think I've found a perfect name for it, but IMO the current choice ofPut
is already taken to mean something else. - use
Put(Sync)
andPut(Deferred)
on prealloced buffers just like on regular buffers.
I had previously put forth these changes in PR #1305. (I'm not asking for it to merged here. It'd need some work and probably would conflict anyway in its old shape). It serves to demonstrate that my suggestion is quite easy straightforward to implement.
Let me try to summarize the arguments against it:
-
Put
can be overloaded, so everything Put-related should be calledPut
. -
Put(Sync)
implies data readiness.Put(Deferred)
implies data pointer readiness, but not data content readiness, which in this case happens atPerformPuts
. HencePut
doesn't actually imply data content readiness. - It's implicitly clear that the new
Put
overload returns a buffer. - The returned buffer is initialized (to zero or some other constant value) at prealloc time, so the buffer is actually put into adios2 at that time.
- Symmetry with Get is broken.
I'm not going to try to rehash the the discussion from #1290 here. I do want to address the symmetry with a future internal-buffer access version of Get
a little bit, but if this turns into a discussion on that interface, I think that also better be done in a separate thread:
- I think there may be a benefit on the
Get
side of things to have an interface that avoids a copy / duplicate memory as well. -
Get
is actually more complicated, because generally speaking, what's returned to the user is a view into the global array, which may well be assembled for a number of blocks that may even live in different files. Once you have to assemble things, you have to copy anyway, in which case the existing interface will do just fine. - However, there is a case where the app may ask for exactly the block of data that was previously written (as one unit) into the file, and on the same proc. That may actually be an important use case, as checkpoint / restart would often see this pattern.
- I think
Get(Sync)
would be the right name for a function returning something Span like, with the expectation that the data will be accessible in the returned Span after theGet(Sync)
returns. - I also think there should be a
Get(Deferred)
for internal buffer access, that would also return a Span, but with the expectation that the data is not yet available, but only afterPerformGets
. Same semantics as the current API. This would be useful if, for example, the data to be read actually lives on a different proc, so someMPI_Irecv()
could be started, but not yetMPI_Wait
ed for. - In either case, when using the internal buffer, there needs to be a mechanism to tell the library that the user is done with using the preallocated data, so that the library can free or reuse the buffer. Something like
GetRelease
(that's what I just came up with, hopefully someone else will come up with a better choice.)
So what I'm getting at is that there's is no perfect symmetry between Put and Get, there already is not in the existing case. (Sure, there's Get / PerformGets and Put / PerformPuts. But the underlying semantics is actually quite different, as to when the data is ready, e.g., One takes a T*, the other a const T*, and for good reason.) An internal-buffer based Get should maintain the semantics of Get/PerformGets, but not expect to get add an additional function, just like on the Put side.