[gpu_operations] add support for batched memory copies in GPUAllgather

[kvignesh1420]

Checklist before submitting

• Did you read the contributor guide? YES
• Did you update the docs? Not yet. Open to suggestions for an appropriate doc to update.
• Did you write any tests to validate this change? Existing GPU allgather tests validate the logic
• Did you update the CHANGELOG, if this change affects users? There are no user facing modifications.

Description

This PR builds on the work of #2435 by switching to a batched device-to-device memory copies approach for GPUAllgather::MemcpyInFusionBuffer and GPUAllgather::MemcpyOutFusionBuffer. The behavior can be reverted back by setting HOROVOD_BATCH_D2D_MEMCOPIES=false which is similar to the existing GPUAllreduce implementation.

Review process to land

1. All tests and other checks must succeed.
2. At least one member of the technical steering committee must review and approve.
3. If any member of the technical steering committee requests changes, they must be addressed.

[chongxiaoc]

Hi @romerojosh , can you help review this?

[github-actions]

Unit Test Results

     923 files  +  30       923 suites  +30   9h 50m 52s ⏱️ - 1m 3s
     781 tests ±    0       737 ✔️ ±    0       44 💤 ±    0  0 ❌ ±0 
19 807 runs  +746  14 132 ✔️ +466  5 675 💤 +280  0 ❌ ±0 

Results for commit 9568c46. ± Comparison against base commit b67d756.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Results (with flaky tests)

  1 104 files  +     69    1 104 suites  +69   10h 49m 56s ⏱️ + 26m 36s
     781 tests ±       0       735 ✔️  -     2       44 💤 ±    0  2 ❌ +2 
23 718 runs  +1 351  16 417 ✔️ +823  7 299 💤 +526  2 ❌ +2 

For more details on these failures, see this check.

Results for commit 9568c46. ± Comparison against base commit b67d756.

♻️ This comment has been updated with latest results.

[romerojosh]

LGTM! 👍
Thanks for the great contribution @kvignesh1420!

[kvignesh1420]

@romerojosh thanks for the review. As a follow up of this PR, what do you think about a new BatchedD2DParams struct which can handle more than 160 entries at once? (maybe a struct with pointer entries instead of the arrays themselves?) I observed that the limitation of 160 entries per batch is due to the size restriction of 4KB for formal parameters to the cuda kernel. I am not sure if the performance benefit would be significant but I am happy to discuss this further if you are interested.

cc: @chongxiaoc

[romerojosh]

@kvignesh1420 The benefit of using the struct with arrays rather than pointers is that it removes the need to use any additional GPU memory allocations or memcopies of the parameters, as we can pass the struct directly into the kernel as an argument (so long as the argument is under 4KB as you've discovered).

[kvignesh1420]

@romerojosh that makes sense 👍

[maxhgerlach]

GPU tests on Buildkite failed for this PR because there were problems cloning the Eigen repository at the time (https://buildkite.com/horovod/horovod/builds/7951#0181bd82-600d-455e-9ce6-eba81da4e564):

fatal: unable to access 'https://gitlab.com/cantonios/eigen.git/': The requested URL returned error: 503
Failed to clone 'third_party/eigen' a second time, aborting

I've re-triggered the CI (Results) workflow, hopefully it will go through now: https://github.com/horovod/horovod/actions/runs/2600228008

[kvignesh1420]

GPU tests on Buildkite failed for this PR because there were problems cloning the Eigen repository at the time (https://buildkite.com/horovod/horovod/builds/7951#0181bd82-600d-455e-9ce6-eba81da4e564):

fatal: unable to access 'https://gitlab.com/cantonios/eigen.git/': The requested URL returned error: 503
Failed to clone 'third_party/eigen' a second time, aborting

I've re-triggered the CI (Results) workflow, hopefully it will go through now: https://github.com/horovod/horovod/actions/runs/2600228008

@maxhgerlach seems like the tests failed again due to gpg key retrieval timeout:
https://buildkite.com/horovod/horovod/builds/7970#0181d499-2c8b-4e58-a9d8-ddfb32e9c749

[maxhgerlach]

@maxhgerlach seems like the tests failed again due to gpg key retrieval timeout: https://buildkite.com/horovod/horovod/builds/7970#0181d499-2c8b-4e58-a9d8-ddfb32e9c749

No reason to worry for now: those passed on retry. I think we are good to merge.