Gpuweb: Tweaks to the `mapAsync` function.

Created on 20 May 2020  路  6Comments  路  Source: gpuweb/gpuweb

While discussing #708 there was some concern by @kvark that mapAsync didn't show clearly in the place where it was called if it was for reading and writing, and given the semantics of the two are very different, this can be confusing.

The two proposed solutions are:

  • Splitting into two different entrypoints mapReadAsync and mapWriteAsync:
partial interface GPUBuffer {
    Promise<void> mapReadAsync(
        optional GPUSize64 offset = 0,
        optional GPUSize64 size = 0);
    Promise<void> mapWriteAsync(
         optional GPUSize64 offset = 0,
         optional GPUSize64 size = 0);
    // getMappedRange and unmap are unchanged
}
  • Adding map flags (@jdashg's idea) that could later be used to extend mapAsync with more modes:
typedef [EnforceRange] unsigned long GPUMappingFlags;
interface GPUMapping {
    const GPUMappingFlags READ  = 0x01;
    const GPUMappingFlags WRITE = 0x02;
};

partial interface GPUBuffer {
    Promise<void> mapAsync(
         GPUMappingFlags flags,
         optional GPUSize64 offset = 0,
         optional GPUSize64 size = 0);
}

I think either solution would work well, and don't have an opinion either way, but resolution of this is blocking our implementation of the new mapping so it would be nice to decide.

+CC @kvark @jdashg @litherum @kainino0x @RafaelCintron

Most helpful comment

Also note that mapWriteAsync() doesn't need arguments, they aren't useful.

They are because in the case where we can't import shmem on the GPU, the bounds on mapWriteAsync tell us exactly what needs to be sent to the GPU process.

All 6 comments

It makes very little difference to me. Preference for mapReadAsync/mapWriteAsync because it's much easier to type. The latter is good if we want to extend it someday, but I'm not sure how likely that is. (READ|WRITE, READ|PERSISTENT, etc.)

Also note that mapWriteAsync() doesn't need arguments, they aren't useful.
At the same time, if we are to add flags, we'll likely want them on the mapWriteAsync path. For example, we could have a "discard" mode where it's forced to give you zeroed contents, and it overwrites whatever was at the range. So I guess there is a mixed choice (3) here:

enum GPUMapMode {
  "wait",
  "discard",
};
partial interface GPUBuffer {
    Promise<void> mapReadAsync(
        optional GPUSize64 offset = 0,
        optional GPUSize64 size = 0);
    Promise<void> mapWriteAsync(optional GPUMapMode mode = "wait");
    // getMappedRange and unmap are unchanged
}

I wonder if adding this GPUMapMode mode argument at a later point is backwards compatible. If that's the case, I think having mapWriteAsync() now is best.

Adding a new optional argument to the end of a method is backward compatible.

Also note that mapWriteAsync() doesn't need arguments, they aren't useful.

They are because in the case where we can't import shmem on the GPU, the bounds on mapWriteAsync tell us exactly what needs to be sent to the GPU process.

I had a professor that hated multi-mode functions so I vote for separate functions + @kvark's point that the arguments could be very different. What are some other possible modes? RW?

Moving to "Needs Specification" as we agreed on the group call to proceed with option (2) for future-proofing.

Was this page helpful?
0 / 5 - 0 ratings