Skip to content

Commit 3fc4e10

Browse files
aendkggerganovJohannesGaessler
authored
sched : reintroduce less synchronizations during split compute (ggml-org#20793)
* CUDA: Improve performance via less synchronizations between token (ggml-org#17795) * Adds CPU-to-CUDA copy capability to ggml_backend_cuda_cpy_tensor_async() * Adds function to relax sync requirements between input copies on supported backends (CUDA for now) * Exchanges synchronous copy with async copy function. * Adds macro guards to allow compilation in non-CUDA builds * Reworked backend detection in ggml-backend.cpp to avoid linking conflicts * Relax requirement of checks in async CUDA copies from backend and buffer type to just buffer type, to avoid linking issues * Minor cleanup * Makes opt-in to relax use of explicit syncs more general. Backends like vulkan which require a synchronization between HtoD copies and graph execution could also adopt this change now. * Reintroduces stricter check for CPU->CUDA backend async copy via GGML_DEVICE_TYPE_CPU. * Corrects initialization of ggml_backend_sync_mode in ggml_backend_sched_split initialization * Simplifies synchronizations to adhere to `saaasg` pattern. * Apply suggestion from @ggerganov (src->buffer to buf_src) Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * Apply suggestion from @ggerganov (src->buffer to buf_src) v2 Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * Apply suggestions from @JohannesGaessler code review Co-authored-by: Johannes Gäßler <johannesg@5d6.de> * Adds single-GPU synchronizations to multi-GPU settings to fix hip backend pipeline parallel bugs. * Scheduler Hardening: Exclude hip/MUSA from copy_from_host CPU split -> GPU split optimization * Scheduler Hardening: Re-adding original additional synchronizations for non-async backends * Adds disclaimer to hip/musa exclusion of copy_from_host. Highlights that it is out of precaution, but that no perf-impact is visible, and that it can be revisited separately anytime. --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Johannes Gäßler <johannesg@5d6.de>
1 parent 5d8ccdf commit 3fc4e10

2 files changed

Lines changed: 27 additions & 7 deletions

File tree

ggml/src/ggml-backend.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,8 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
15511551
int split_backend_id = split->backend_id;
15521552
ggml_backend_t split_backend = sched->backends[split_backend_id];
15531553

1554+
ggml_backend_synchronize(split_backend);
1555+
15541556
// copy the input tensors to the split backend
15551557
for (int input_id = 0; input_id < split->n_inputs; input_id++) {
15561558
ggml_backend_t input_backend = ggml_backend_sched_get_tensor_backend(sched, split->inputs[input_id]);
@@ -1561,15 +1563,15 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
15611563
// inputs from the user must be copied immediately to prevent the user overwriting the data before the copy is done
15621564
if (sched->events[split_backend_id][sched->cur_copy] != NULL) {
15631565
ggml_backend_event_synchronize(sched->events[split_backend_id][sched->cur_copy]);
1564-
} else {
1566+
} else if (!split_backend->iface.cpy_tensor_async) {
15651567
ggml_backend_synchronize(split_backend);
15661568
}
1567-
ggml_backend_tensor_copy(input, input_cpy);
1569+
ggml_backend_tensor_copy_async(input_backend, split_backend, input, input_cpy);
15681570
} else {
15691571
// wait for the split backend to finish using the input before overwriting it
15701572
if (sched->events[split_backend_id][sched->cur_copy] != NULL) {
15711573
ggml_backend_event_wait(split_backend, sched->events[split_backend_id][sched->cur_copy]);
1572-
} else {
1574+
} else if (!split_backend->iface.cpy_tensor_async) {
15731575
ggml_backend_synchronize(split_backend);
15741576
}
15751577

@@ -1674,6 +1676,8 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
16741676
}
16751677
}
16761678

1679+
ggml_backend_synchronize(split_backend);
1680+
16771681
if (!sched->callback_eval) {
16781682
enum ggml_status ec = ggml_backend_graph_compute_async(split_backend, &split->graph);
16791683
if (ec != GGML_STATUS_SUCCESS) {

ggml/src/ggml-cuda/ggml-cuda.cu

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3192,11 +3192,24 @@ static bool ggml_backend_cuda_cpy_tensor_async(ggml_backend_t backend_src, ggml_
31923192
ggml_backend_buffer_t buf_src = src->view_src ? src->view_src->buffer : src->buffer;
31933193
ggml_backend_buffer_t buf_dst = dst->view_src ? dst->view_src->buffer : dst->buffer;
31943194

3195-
if (!ggml_backend_is_cuda(backend_src) || !ggml_backend_is_cuda(backend_dst)) {
3195+
// Enables async copies from CPU to CUDA, instead of only CUDA-to-CUDA
3196+
// Excluding this path for HIP and MUSA as a precaution.
3197+
// According to the summary in https://github.com/ggml-org/llama.cpp/pull/20793#issuecomment-4275794315, this change is not beneficial for hip anyways.
3198+
// Additionally, there is a lot of anectodal evidence that hip/musa stream behavior might not always 1:1 match CUDA behavior.
3199+
// e.g. https://github.com/ROCm/rocm-systems/issues/5109
3200+
// It thus makes sense to exclude this path for HIP and MUSA. This PR was not aimed these backends, the majority of testing happened on CUDA.
3201+
// This can be revisited in the future if enabling copy_from_host benefits hip/MUSA, and if the PR author can extensively test on these backends.
3202+
#if defined(GGML_USE_HIP) || defined(GGML_USE_MUSA)
3203+
const bool copy_from_host = false;
3204+
#else
3205+
const bool copy_from_host = ggml_backend_buffer_is_host(buf_src) && ggml_backend_dev_type(backend_src->device) == GGML_BACKEND_DEVICE_TYPE_CPU;
3206+
#endif
3207+
3208+
if (!(copy_from_host || ggml_backend_is_cuda(backend_src)) || !ggml_backend_is_cuda(backend_dst)) {
31963209
return false;
31973210
}
31983211

3199-
if (!ggml_backend_buffer_is_cuda(buf_src) || !ggml_backend_buffer_is_cuda(buf_dst)) {
3212+
if (!(copy_from_host || ggml_backend_buffer_is_cuda(buf_src)) || !ggml_backend_buffer_is_cuda(buf_dst)) {
32003213
return false;
32013214
}
32023215

@@ -3207,14 +3220,17 @@ static bool ggml_backend_cuda_cpy_tensor_async(ggml_backend_t backend_src, ggml_
32073220
ggml_backend_cuda_buffer_context * buf_ctx_src = (ggml_backend_cuda_buffer_context *) buf_src->context;
32083221
ggml_backend_cuda_buffer_context * buf_ctx_dst = (ggml_backend_cuda_buffer_context *) buf_dst->context;
32093222

3210-
if (cuda_ctx_src->device != buf_ctx_src->device || cuda_ctx_dst->device != buf_ctx_dst->device) {
3223+
if ((copy_from_host && cuda_ctx_dst->device != buf_ctx_dst->device) ||
3224+
!copy_from_host && (cuda_ctx_src->device != buf_ctx_src->device || cuda_ctx_dst->device != buf_ctx_dst->device)) {
32113225
#ifndef NDEBUG
32123226
GGML_LOG_DEBUG("%s: backend and buffer devices do not match\n", __func__);
32133227
#endif // NDEBUG
32143228
return false;
32153229
}
32163230

3217-
if (backend_src != backend_dst) {
3231+
if (copy_from_host) {
3232+
CUDA_CHECK(cudaMemcpyAsync(dst->data, src->data, ggml_nbytes(dst), cudaMemcpyHostToDevice, cuda_ctx_dst->stream()));
3233+
} else if (backend_src != backend_dst) {
32183234
// copy on src stream
32193235
if (cuda_ctx_src->device == cuda_ctx_dst->device) {
32203236
CUDA_CHECK(cudaMemcpyAsync(dst->data, src->data, ggml_nbytes(dst), cudaMemcpyDeviceToDevice, cuda_ctx_src->stream()));

0 commit comments

Comments
 (0)