From c262c4b3e8d93f688952999db33fbcb556d7242f Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Thu, 2 Apr 2026 02:11:55 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20restore=20EventTrackingGuard=20every=20s?= =?UTF-8?q?tep=20=E2=80=94=20keep=20check=5Ferr,=20remove=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hang was caused by cudarc accumulating stale CudaEvent errors from CUDA Graph replays. EventTrackingGuard + check_err must run every step to drain these errors. The cuStreamSynchronize is only needed on the very first call to clear the initial backlog from graph capture — subsequent steps rely on stream ordering. Guard (~100ns) + check_err (~50ns) per step is negligible. The cuStreamSynchronize (~150µs pipeline drain) is eliminated on all but the first step. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ml/src/cuda_pipeline/gpu_dqn_trainer.rs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs b/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs index 914038133..7c332e926 100644 --- a/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs +++ b/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs @@ -1991,16 +1991,16 @@ impl GpuDqnTrainer { let sh2 = self.config.shared_h2; let n_bytes = batch_size * sh2 * f32_size; - // Disable event tracking for all buffer access in this method. - // After CUDA Graph capture, cudarc's device_ptr() records write events - // that fail with CUDA_ERROR_INVALID_VALUE on stale captured graph events. - // First call syncs to clear stale events; subsequent calls rely on stream ordering. + // Disable event tracking every step — cudarc records write events on + // device_ptr() calls that conflict with CUDA Graph-captured buffers. + // Guard is ~100ns. Only the first call needs cuStreamSynchronize to + // clear initial stale events from graph capture. + let _evt_guard = EventTrackingGuard::new(self.stream.context()); + let _ = self.stream.context().check_err(); if !self.attention_initialized { unsafe { cudarc::driver::sys::cuStreamSynchronize(self.stream.cu_stream()); } - let _ = self.stream.context().check_err(); self.attention_initialized = true; } - let _evt_guard = EventTrackingGuard::new(self.stream.context()); // Run attention forward: save_h_s2 → attention output buffer let attn_output = attention.forward(&self.save_h_s2, batch_size)?; @@ -5490,13 +5490,16 @@ impl GpuDqnTrainer { target_b: &mut BranchingWeightSet, tau: f32, ) -> Result<(), MLError> { - // Sync stream and clear any stale errors from graph capture phase. - // First call only: clear stale cudarc events from graph capture phase. - // Subsequent calls skip the sync — stream ordering is sufficient. + // Disable event tracking every step — cudarc records write events on + // device_ptr() calls that conflict with CUDA Graph-captured buffers. + // The guard is cheap (~100ns). The cuStreamSynchronize was the bottleneck + // and is NOT needed — stream ordering guarantees correctness. + let _eg = EventTrackingGuard::new(self.stream.context()); + let _ = self.stream.context().check_err(); + + // First call: flatten target weights into flat buffer (20 DtoD copies, once only). if !self.target_params_initialized { - let _eg = EventTrackingGuard::new(self.stream.context()); unsafe { cudarc::driver::sys::cuStreamSynchronize(self.stream.cu_stream()); } - let _ = self.stream.context().check_err(); self.flatten_target_weights(target_d, target_b)?; self.target_params_initialized = true; }