Relying on this can be error-prone.

TODO: If an actor sends to itself, the current schedule is correct becausetask_idxs containSEND andRECV in order. The order is provided from the topological sort.

Maybe use something better thanrecord_stream.

To be improved: Sending GPU future across actors forces a blocking wait.

Related: Currently, overlapping is done by immediately launching NCCL operations when their inputs are ready. The downstream task that reads the NCCL operation's output will get a GPU future and wait on it.

Limitations:

1. Futures can't be sent across actors, in which case waiting can result in worse performance than executing the DAG withoverlap_gpu_communication=False.
2. Even when futures are only read by the same actor locally, if the actor does multiple NCCL recv operations, they will be scheduled as the first operations in the execution schedule. Therecv_stream.synchronize() call in_NcclGroup.recv blocks the CPU on the second recv operation. (TODO: Determine if the synchronization is still needed.)

1. We observe that when futures are sent across actors, overlap is slightly slower than non-overlap version. This is because: (1) There is no benefit from overlapping when futures are sent across actors; (2) The exact stream sync is different in overlap/non-overlap.

2. Do we need different schedules for overlap/non-overlap? We change schedules and move P2P early in overlap.

3. Do we actually need to sync before launching another NCCL read/write?

4. When an actor have multiple NCCL reads, these NCCL reads are not perfectly overlapped since there is sync between each NCCL read.

1. We observe that when futures are sent across actors, overlap is slightly slower than non-overlap version. This is because: (1) There is no benefit from overlapping when futures are sent across actors; (2) The exact stream sync is different in overlap/non-overlap.

2. Do we need different schedules for overlap/non-overlap? We change schedules and move P2P early in overlap.

3. Do we actually need to sync before launching another NCCL read/write?

4. When an actor have multiple NCCL reads, these NCCL reads are not perfectly overlapped since there is sync between each NCCL read.

Instead of adding this extra logic here, it would be better if we can make sureself.input_channels is always empty forself.requires_nccl_read when reaching this line. Same forself.output_channels andself.requires_nccl_write.

Nice fix here! Can you separate out the fix for CUDA event destruction into a separate PR? This will be faster to merge and less likely to get reverted.

This naming is slightly confusing because it sounds likecontext should be passed as an argument to this function. Maybe justexec_operation and_exec_operation_without_context for the inner method.

This looks much better!

Does this mean that the send and recv nodes will get the same bind index as the original DAG node? Will it be a problem that multiple nodes have the same bind index?

I think this is quite brittle and difficult to understand. It would be better if we can think of a way to make all of the bind indices for a given actor unique.

Either we need to compute the new bind indices when creating the CompiledDAG or we need a way to create the NCCL nodes while the user is creating the initial DAG.