Record dispatches into one batch and submit when a result is read

CONTRIBUTING.md

@@ -59,6 +59,26 @@ git-ignored — do not commit them.
 parity yet). Please add cases there for any behavior you change or add, and make
 sure it prints `ALL TESTS PASSED` before opening a PR.
 
+`test/shader_error_test.rb` is separate because it has to break the GPU context,
+which is a process-wide singleton — running it alongside the main suite would
+leave every later test without pipelines. Run it on its own:
+
+```sh
+./build/host/bin/mruby test/shader_error_test.rb            # missing .spv files
+./build/host/bin/mruby test/shader_error_test.rb <bad-dir>  # .spv the driver rejects
+```
+
+## Error handling
+
+Every Vulkan call that returns a `VkResult` goes through `VK_CHECK`, which
+raises an mruby exception instead of leaving an unusable handle behind. Please
+keep new calls checked — an unchecked failure surfaces either as a crash of the
+whole VM or, worse, as a plausible-looking wrong number.
+
+Because `mrb_raise` unwinds, anything already allocated has to be released
+*before* the check. In particular, wrap a new `GpuBuffer` with `wrap_buffer`
+before the dispatch that fills it, so the GC owns it if the dispatch raises.
+
 ## Reporting issues
 
 GPU behavior is environment-specific, so please include:

README.md

@@ -47,7 +47,7 @@ built on it:
 | Spectral | `#rfft` → `GPU::SComplex`; `#magnitude(k)`, `#power_spectrum(k)` |
 | Host transfer | `#to_a`, `#head(k)` |
 | Metadata | `#size` / `#length`, `#shape`, `#ndim` |
-| Device | `GPU.info`, `GPU.device_name`, `GPU.init(dir)` |
+| Device | `GPU.info` (device, API version, backend, `:max_workgroups`), `GPU.device_name`, `GPU.init(dir)` |
 
 Data lives in a `VkBuffer` the whole time; the only host copies happen in `#to_a` /
 `#head`. Arithmetic, reduction and the FFT are Vulkan compute dispatches.
@@ -132,7 +132,8 @@ GPU::SFloat (mruby)                         mrblib/gpu_narray.rb  (shape, mean,
    ▼
 src/gpu_narray.c    dtype methods; picks a pipeline; keeps results on the GPU
    ▼
-src/gpu_vulkan.c    dispatch_compute(): descriptor set → command buffer → submit → fence
+src/gpu_vulkan.c    dispatch_pipeline(): descriptor set → recorded into one command buffer, + barrier
+                    gpu_flush():         submit → fence, once per batch, when a result is read
    ▼
 shader/*.comp       add / sub / mul / div / scale / adds / sum
                     fft_bitrev / fft_stage / cmag            (GLSL → SPIR-V)
@@ -150,10 +151,67 @@ buffer of `2n` floats (interleaved re/im, wrapped as `GPU::SComplex`):
    order, so the butterflies afterwards run in natural order and in place.
 2. `fft_stage` runs once per pass with a doubling half-size `h = 1, 2, … n/2`. Each
    invocation owns one butterfly, and a pass's pairs partition `[0, n)` exactly — so
-   the in-place writes don't race, and one dispatch per pass is the barrier between
-   passes.
+   the in-place writes don't race, and the memory barrier recorded after each pass
+   orders it before the next.
 3. `cmag` reduces the spectrum to a real `GPU::SFloat` of magnitudes or powers.
 
+### Deferred submission
+
+A dispatch is not a round trip. Every operation is *recorded* into one long-lived
+command buffer, followed by a memory barrier so the next dispatch sees its writes,
+and nothing is submitted until the host actually needs a result. Then everything
+recorded so far goes to the GPU in one `vkQueueSubmit`, with one fence wait.
+
+```ruby
+b = a * 2 + 1 - 3 + 4     # four dispatches recorded, nothing submitted
+GPU.pending               #=> 4
+b.to_a                    # one submit, one wait, then the copy
+GPU.pending               #=> 0
+```
+
+The sync points, where the batch is flushed:
+
+- reading a result (`to_a`, `head`, `sum`, `mean`, `inspect`, …);
+- a host write (`seq`, `fill`) to a buffer that a queued dispatch reads or writes —
+  a buffer the batch never touched is written at once;
+- the descriptor pool running dry (256 dispatches), which just flushes early;
+- `GPU.sync`, explicitly — useful when timing a batch;
+- shutdown.
+
+A buffer whose Ruby object is collected while a queued dispatch still references it
+is kept until that batch has run, then freed. `rfft` benefits without changes: its
+`1 + log2(n)` passes are one submit.
+
+`GPU.sync_mode = :eager` restores a submit and a wait after every dispatch — the
+behaviour this library had before batching, kept so the two can be measured
+against each other (`GPU.sync_mode` reads it back; the default is `:deferred`).
+
+| Apple M5, 100-run mean | `a * 2 + 1 - 3 + 4` (4 dispatches) | `power_spectrum` |
+|---|---|---|
+| 1,024 elements, eager (4 / 12 waits) | 0.95 ms | 2.69 ms |
+| 1,024 elements, deferred (1 wait) | 0.25 ms | 0.29 ms |
+| 1,048,576 elements, eager | 2.33 ms | 7.13 ms |
+| 1,048,576 elements, deferred | 2.12 ms | 2.74 ms |
+
+At 1M elements the four passes over memory dominate, and batching alone cannot
+remove those — that is what fusing the expression into one shader is for
+([mruby-gpu-kernel](https://github.com/yujiteshima/mruby-gpu-kernel): 0.35 ms).
+
+Add-on gems can put their own pipelines into the same batch through
+`dispatch_pipeline()` in `src/gpu_internal.h`.
+
+### Limits and failures
+
+One operation is one dispatch, covering 256 elements per workgroup up to the
+device's `maxComputeWorkGroupCount` — `GPU.info[:max_workgroups] * 256`
+elements, about 16.7M where that limit is 65535. Larger arrays raise rather
+than leaving the answer up to the driver.
+
+Failed Vulkan calls raise `RuntimeError` with the call and the `VkResult`.
+A missing shader and a shader the driver rejects are reported differently: the
+first tells you to build the shaders, the second tells you not to bother,
+because the SPIR-V was already there.
+
 ## Roadmap
 
 - **FFT growth**: the radix-2 transform above is in place. Next: an inverse transform,

src/gpu_buffer.c

@@ -5,10 +5,18 @@
  * Raspberry Pi 5 the GPU memory is unified, so a mapped pointer is a cheap
  * view of the same bytes the shader reads/writes.
  *
- * Two ownership models share one GpuBuffer type:
- *   - wrap_buffer()   -> owned by a Ruby object, freed by the GC finalizer.
- *   - create_buffer() + destroy_buffer() -> caller-managed scratch buffers
- *     (used for reduction partials that never become Ruby objects).
+ * Every buffer ends up owned by a Ruby object and freed by the GC finalizer.
+ * Callers wrap as soon as create_buffer returns, before anything that can
+ * raise, so a Vulkan failure unwinds without stranding GPU memory.
+ * destroy_buffer is the finalizer itself, plus create_buffer's own unwind.
+ *
+ * With deferred submission a buffer can be referenced by a command buffer that
+ * has not run yet. Two rules keep that safe:
+ *   - map_buffer flushes the batch first if it touches this buffer, so the
+ *     host never reads a result that is still queued, and never overwrites an
+ *     input that a queued dispatch has yet to read.
+ *   - destroy_buffer parks such a buffer in the graveyard instead of freeing
+ *     it; gpu_flush frees the graveyard once the batch has completed.
  */
 #include "gpu_internal.h"
 
@@ -19,11 +27,18 @@ static void gpu_buffer_free(mrb_state *mrb, void *p) {
 
 const struct mrb_data_type gpu_buffer_type = {"GPU::NArray", gpu_buffer_free};
 
-/* ---- Create a host-visible FP32 buffer of n elements ---- */
+/* ---- Create a host-visible FP32 buffer of n elements ----
+ *
+ * Raises rather than returning a half-built buffer. Because a raise unwinds
+ * out of this function, each step releases what earlier steps created before
+ * handing the failure to gpu_check. */
 GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) {
   GpuBuffer *buf = mrb_malloc(mrb, sizeof(GpuBuffer));
   buf->n = n;
+  buf->buffer = VK_NULL_HANDLE;
+  buf->memory = VK_NULL_HANDLE;
   buf->bytes = sizeof(float) * (VkDeviceSize)n;
+  buf->epoch = 0;   /* never bound; no batch can be waiting on it */
   if (buf->bytes == 0) buf->bytes = sizeof(float); /* avoid zero-size allocation */
 
   VkBufferCreateInfo bi = {
@@ -32,39 +47,85 @@ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) {
     .usage = VK_BUFFER_USAGE_STORAGE_BUFFER_BIT,
     .sharingMode = VK_SHARING_MODE_EXCLUSIVE
   };
-  vkCreateBuffer(g_ctx.device, &bi, NULL, &buf->buffer);
+  VkResult r = vkCreateBuffer(g_ctx.device, &bi, NULL, &buf->buffer);
+  if (r != VK_SUCCESS) {
+    mrb_free(mrb, buf);
+    gpu_check(mrb, r, "vkCreateBuffer");
+  }
 
   VkMemoryRequirements req;
   vkGetBufferMemoryRequirements(g_ctx.device, buf->buffer, &req);
 
+  /* The mapped pointer is how every host read and write reaches this buffer,
+   * so a memory type that is not both host-visible and host-coherent is no
+   * use. Defaulting to index 0 when none matches would allocate from an
+   * arbitrary heap and turn every later map_buffer into a NULL dereference. */
   VkPhysicalDeviceMemoryProperties mem_props;
   vkGetPhysicalDeviceMemoryProperties(g_ctx.physical_device, &mem_props);
-  uint32_t mem_idx = 0;
+  const VkMemoryPropertyFlags want =
+    VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
+  uint32_t mem_idx = UINT32_MAX;
   for (uint32_t i = 0; i < mem_props.memoryTypeCount; i++) {
-    if ((req.memoryTypeBits & (1 << i)) &&
-        (mem_props.memoryTypes[i].propertyFlags &
-         (VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) ==
-         (VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) {
+    if ((req.memoryTypeBits & (1u << i)) &&
+        (mem_props.memoryTypes[i].propertyFlags & want) == want) {
       mem_idx = i;
       break;
     }
   }
+  if (mem_idx == UINT32_MAX) {
+    vkDestroyBuffer(g_ctx.device, buf->buffer, NULL);
+    mrb_free(mrb, buf);
+    mrb_raise(mrb, E_RUNTIME_ERROR,
+      "no host-visible, host-coherent memory type is available for a storage buffer");
+  }
 
   VkMemoryAllocateInfo ai = {
     .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
     .allocationSize = req.size,
     .memoryTypeIndex = mem_idx
   };
-  vkAllocateMemory(g_ctx.device, &ai, NULL, &buf->memory);
-  vkBindBufferMemory(g_ctx.device, buf->buffer, buf->memory, 0);
+  r = vkAllocateMemory(g_ctx.device, &ai, NULL, &buf->memory);
+  if (r != VK_SUCCESS) {
+    vkDestroyBuffer(g_ctx.device, buf->buffer, NULL);
+    mrb_free(mrb, buf);
+    gpu_check(mrb, r, "vkAllocateMemory");
+  }
+
+  r = vkBindBufferMemory(g_ctx.device, buf->buffer, buf->memory, 0);
+  if (r != VK_SUCCESS) {
+    vkFreeMemory(g_ctx.device, buf->memory, NULL);
+    vkDestroyBuffer(g_ctx.device, buf->buffer, NULL);
+    mrb_free(mrb, buf);
+    gpu_check(mrb, r, "vkBindBufferMemory");
+  }
 
   return buf;
 }
 
-/* ---- Free a buffer's GPU resources + the struct itself ---- */
+/* ---- Free a buffer's GPU resources + the struct itself ----
+ *
+ * Runs as the GC finalizer, so it must not raise and must not block: a buffer
+ * the pending batch still references is parked in the graveyard and freed by
+ * the next gpu_flush, after the GPU is done with it. */
 void destroy_buffer(mrb_state *mrb, GpuBuffer *buf) {
   if (!buf) return;
   if (g_ctx.initialized) {
+    if (g_ctx.batch_recording && buf->epoch == g_ctx.epoch) {
+      if (g_ctx.graveyard_len == g_ctx.graveyard_cap) {
+        size_t cap = g_ctx.graveyard_cap ? g_ctx.graveyard_cap * 2 : 64;
+        GpuBuffer **g = realloc(g_ctx.graveyard, cap * sizeof(GpuBuffer *));
+        if (!g) {
+          /* Cannot park it and must not free it under the GPU's feet. Leaking
+           * one buffer is the least bad outcome inside a finalizer. */
+          fprintf(stderr, "mruby-gpu-narray: out of memory deferring a buffer free; leaking it\n");
+          return;
+        }
+        g_ctx.graveyard = g;
+        g_ctx.graveyard_cap = cap;
+      }
+      g_ctx.graveyard[g_ctx.graveyard_len++] = buf;
+      return;
+    }
     vkDestroyBuffer(g_ctx.device, buf->buffer, NULL);
     vkFreeMemory(g_ctx.device, buf->memory, NULL);
   }
@@ -77,10 +138,21 @@ mrb_value wrap_buffer(mrb_state *mrb, struct RClass *klass, GpuBuffer *buf) {
   return mrb_obj_value(data);
 }
 
-/* ---- Host mapping helpers (coherent memory, no flush needed) ---- */
-float *map_buffer(GpuBuffer *buf) {
+/* ---- Host mapping helpers (coherent memory, no flush needed) ----
+ *
+ * Never returns NULL: every caller writes through the pointer immediately, so
+ * an unreported map failure would be a NULL dereference inside the VM.
+ *
+ * This is the sync point of the whole library: the host is about to look at
+ * (or change) the bytes, so any queued dispatch that reads or writes this
+ * buffer has to run first. A buffer the batch never touched maps at once. */
+float *map_buffer(mrb_state *mrb, GpuBuffer *buf) {
+  gpu_sync_buffer(mrb, buf);
   float *mapped = NULL;
-  vkMapMemory(g_ctx.device, buf->memory, 0, buf->bytes, 0, (void **)&mapped);
+  VK_CHECK(mrb, vkMapMemory(g_ctx.device, buf->memory, 0, buf->bytes, 0, (void **)&mapped));
+  if (!mapped) {
+    mrb_raise(mrb, E_RUNTIME_ERROR, "vkMapMemory returned no pointer");
+  }
   return mapped;
 }