-
Notifications
You must be signed in to change notification settings - Fork 1
perf: raise the GPU-to-NIC submission rate with channels, QPs, forward pairs #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,13 +47,13 @@ struct GinResourceConfig { | |
| static constexpr int kMinGinContextCnt = 2; | ||
| static constexpr int kMaxGinContextCnt = kMaxGinContextBudget; | ||
|
|
||
| // Default context count (== default QP count). 11 contexts -> 21 signals/context. | ||
| // Default context count (== default QP count). 13 contexts -> 17 signals/context. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the biggest change, it change the sharing mode from _GPU to _CTA on 12SM configuration. this need to be better highlight in the commit message. |
||
| // Contexts and signals-per-context are inversely coupled through | ||
| // `gin_indexed_signals_for`, so more QPs means a smaller per-context signal budget. The | ||
| // equivalent alternatives are {5, 6, 7, 8, 9, 14}; everything else loses a part somewhere. | ||
| // Notably 12, 15, 16 and 17 all drop to 3 parts at 12 SMs -- 17 (the provider maximum) leaves | ||
| // only 13 signals/context, and its per-SM QP split puts 4 channels on the busiest QP. | ||
| static constexpr int kDefaultGinContextCnt = 11; | ||
| static constexpr int kDefaultGinContextCnt = 13; | ||
|
|
||
| // Per-context indexed-signal budget, workaround for current limitations in provider. | ||
| __forceinline__ __device__ __host__ constexpr int gin_indexed_signals_for(int gin_context_cnt) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You basically deprecated this flag and changed the customer facing API. I am fine with it, but now the perf base line that we compare against is not the same, saying that we reach the same performance as official DeepEP on IB is not correct as they set this flag to false.
We need to make it clear that this flag is no longer behave the same when we publish those results both internally and externally.