ggml: asynchronous scheduler memory copies#25319
Conversation
github-actions
Bot
added
ggml
| for (int split_id = 0; split_id < sched->n_splits; split_id++) { | ||
| struct ggml_backend_sched_split * split = &splits[split_id]; | ||
| int split_backend_id = split->backend_id; | ||
| ggml_backend_t split_backend = sched->backends[split_backend_id]; | ||
|
|
||
| for (int input_id = 0; input_id < split->n_inputs; input_id++) { | ||
| struct ggml_tensor * input = split->inputs[input_id]; | ||
| struct ggml_tensor * input_cpy = tensor_copy(input, split_backend_id, sched->cur_copy); | ||
|
|
||
| if (!(input->flags & GGML_TENSOR_FLAG_INPUT)) { | ||
| continue; | ||
| } | ||
|
|
||
| ggml_backend_tensor_copy_async(split_backend, /*backend_wait =*/ NULL, input, input_cpy); | ||
| any_input_copies[split_backend_id] = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
During an off-site discussion with @am17an he pointed me at a scenario with > 1 splits / backend where pre-fetching inputs like this will potentially overwrite some of them.
|
Vulkan does have working events since #20518, I believe. But our main issue has always been that specifically how each function is meant to work has never been fully specified, so there may be different assumptions on different backends. |
|
I pulled this branch and profiled it locally using Nsight Systems to verify the async memory overlap behavior on my architecture. Environment:
Observations: I've attached the timeline snapshot showing this sequential compute/memory behavior. Could the CUDA Graphs be forcing a synchronization in this specific setup, or is the model size (1.1B) too small to saturate the compute and allow for overlapping? 
Best regards, |
|
Adding some general comments from my side as @aendk is still out this week:
Given the trouble we had with this previously, I really think we should offer and implement isolated tests a backend can use to validate its implementation against the expected behavior. Happy to assist/contribute here if you want.
I don't think this is actually true. If
But CUDA -> CUDA copies in your draft still work via enqueuing an event internally that is not exposed via ggml? Note that this is a feature I want to keep. On second thought, seems we always synchronize
Agreed on the need for async/buffered graph prep + execution. AFAIK currently we synchronize after a batch in llama.cpp (e.g. when extracting logits/the sampled token), and across ubatches we synchronize via #20927 only pipeline parallel when a context would be reused (and we would thus trigger the race condition). |
This is expected to bring perf by removing the unneeded |
5 participants
The ggml backend scheduler works by automatically creating "splits" of a ggml graph that can be per backend based on e.g. weight locations. By necessity some data may need to be copied to a different backend to facilitate this. As of right now these copies are entirely synchronous, #17795 #20793 aimed to improve performance by making these copies partially asynchronous. However, these PRs were eventually reverted due to unintended side effects for come hardware setups. Part of the problem is that the ggml backend interface is incomplete in some places and with Diego on an indefinite break we don't have a ggml-side authority w.r.t. the intended design. This PR is my attempt at finding a consensus among ggml maintainers for how to resolve the incomplete parts and how asynchronous copies should be handled.
ggml Backend API Changes
ggml_backend_event_tcan be passed to anyggml_backend_t. However, there is no mechanism to assert that the event and the backend are actually compatible; in e.g. the CUDA backend any event is simply cast tocudaEvent_twithout any checks for misuse. This PR changes the return types ofggml_backend_event_recordandggml_backend_event_waitto bool to indicate whether the backend could successfully use the event. These changes make it easier to use backends across ggml backend (devices) without risking segfaults.ggml_backend_tensor_copy_asyncautomatically enforces synchronization betweenbackend_srcandbackend_dst. However, this limits the flexibility of this API call. When I implemented-sm tensorDiego suggested to me to separate out the asynchronous copy and the synchronization. For CUDA and HIP this would not at all be a problem since the internal synchronization mechanism is the same as withggml_backend_event_t. However, I think to remember that @0cc4m once informed me that Vulkan does not have native support for events. So what I did with this PR is to instead replacebackend_srcandbackend_dstwith a mandatorybackend_copyand an optionalbackend_wait(if not null it is made to wait for the copy to complete). There is no constraint on the memory locations of thesrcanddsttensors other thatbackend_copyneeds to be able to access them. The change toggml_backend_tensor_copy_asyncis backwards compatible though the only applications are in the ggml backend scheduler and the meta backend anyways.ggml Backend Scheduler Changes
GGML_FLAG_INPUTare assumed to be set statically by the user code and thus do not need any synchronization.GGML_FLAG_INPUThas been copied. Preferably use events for this, full backend synchronizations otherwise. To be clear: this is currently necessary due to what I would consider a misues of the ggml backend scheduler by the llama.cpp user code. Once control is returned to the llama.cpp user code it will to my understanding start overwriting the data of the inputs without first callingggml_backend_sched_synchronize- I really think that it should. Long-term the in my opinion correct solution is to buffer ggml graphs so that the user code can start preparing a graph with separate inputs while the previous one(s) are still being executed.Performance changes
CUDA/HIP performance
For actual, real life use the performance impact of the asynchronous copies is very small, a few % at most. Stories 15M provides an effective upper bound on the reduction in overhead, there the speedup can be 10-30%. Though I don't have the setup for testing it I would assume that the impact for audio models would be quite noticeable.
TODOs
Requirements