Request objects and Waitall! allocations

[simonbyrne]

Our MPI.Request objects are currently mutable structs containing an MPI_Request handle. The problem is that the non-blocking group test/wait functions (such as MPI_Waitall) require a vector of MPI_Request handles: if we give them a vector of MPI.Request objects, we need to convert this somehow.

On the current release version (v0.19.2), MPI.Waitall! just allocates a vector of MPI_Request objects:
https://github.com/JuliaParallel/MPI.jl/blob/v0.19.2/src/pointtopoint.jl#L451

In the current master branch, I defined a RequestSet object, which includes a vector of both: calling setindex! will update both accordingly

MPI.jl/src/nonblocking.jl

Line 204 in d34be21

mutable struct RequestSet <: AbstractVector{Request}

MPI.Waitall! has an extra hook after completion which will go back and update the Request objects:

MPI.jl/src/nonblocking.jl

Line 267 in d34be21

update!(reqs)

An alternative approach would be to simply get rid of the mutable struct altogether (so MPI.Request = MPI_Request). This would avoid the copying and updates, reducing overhead, but at the cost of not having request objects being mutable by MPI.Test and friends: this would mean

• Request objects are no longer freed by the GC (which, to be honest, I don't think is a big loss: I don't think much code actually allows dangling requests).
• It would break any code that relies on checking the status of the request after MPI.Wait or MPI.Test (since we have no way to then mutate those objects).
• Request objects would not be able to hold a reference to the buffer, so users would need to manually keep a reference to prevent it being gc-ed before the request has completed.

This was prompted by @OsKnoth who brought this to my attention, and is related to #639 which proposed a lower-level API

[simonbyrne]

It seems the Request objects themselves allocate: JuliaParallel/MPIBenchmarks.jl#4

[simonbyrne]

I spoke with @vchuravy and @aviatesk: it looks like some of the challenges are that we call a C function in the finalizer, which is tricky to handle.

Ideally what we would have is a mechanism to pass responsibility for cleaning up from the Request to the RequestSet, but it's not clear how one could make that work.

A simpler option might be to pass the Request or a view into a RequestSet to Isend/Irecv, so that it can be directly mutated, rather than handling it via setindex!

[simonbyrne]

Fixed by #677.