-
Notifications
You must be signed in to change notification settings - Fork 0
perf(hybrid): don't split a channel into more parts than it has tokens for #2
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 |
|---|---|---|
|
|
@@ -55,6 +55,20 @@ static constexpr int kMinSubTokensDefault = (EP_MIN_SUB_TOKENS) > 1 ? (EP_MIN_SU | |
| #define EP_SM100_MIN_SUB_TOKENS 15 | ||
| #endif | ||
|
|
||
| // Minimum tokens a scale-out part must carry to be worth its own `flush_part` put, i.e. the | ||
| // part-level analogue of `kMinSubTokensDefault` above. `compute_part_allocation()` only ever | ||
| // caps the part count from ABOVE when the indexed-signal budget is tight, and that budget is | ||
| // loosest exactly when a channel holds the fewest tokens, so small-batch shapes settle on | ||
| // `kMaxParts` -- the worst end of the axis. Set to 1 to disable the clamp entirely, i.e. to | ||
| // restore the previous behaviour exactly (a value of 1 must SHORT-CIRCUIT rather than divide by | ||
| // one: `kNumMaxTokensPerChannel / 1` still clamps whenever a channel holds fewer tokens than the | ||
| // budget allows parts, which is a different geometry from the old code and not a control). | ||
| #ifndef EP_MIN_TOKENS_PER_PART | ||
| #define EP_MIN_TOKENS_PER_PART 15 | ||
| #endif | ||
|
|
||
| static constexpr int kMinTokensPerPart = (EP_MIN_TOKENS_PER_PART) > 1 ? (EP_MIN_TOKENS_PER_PART) : 1; | ||
|
|
||
| template <int kNumSubParts, int kMinSubTokens = kMinSubTokensDefault> | ||
| __device__ __host__ __forceinline__ int num_sub_parts_at(const int& part_tokens) { | ||
| if constexpr (kNumSubParts <= 1) { | ||
|
|
@@ -140,9 +154,16 @@ template <bool kDoCPUSync, | |
| int kNumScaleupRanksPerLane = math::constexpr_ceil_div(kNumScaleupRanks, 32), | ||
| int kNumChannelsPerSM = kNumScaleoutWarps, | ||
| int kNumChannels = kNumScaleoutWarps * kNumSMs, | ||
| int kNumParts = gin_alloc::constexpr_num_parts( | ||
| kNumGinSignals, kNumSMs, kNumQPs, (kNumNotifyWarps > 0), kNumScaleoutWarps), | ||
| int kNumMaxTokensPerChannel = math::constexpr_ceil_div(kNumMaxTokensPerRank, kNumChannels), | ||
| int kNumBudgetParts = gin_alloc::constexpr_num_parts( | ||
| kNumGinSignals, kNumSMs, kNumQPs, (kNumNotifyWarps > 0), kNumScaleoutWarps), | ||
| // NOTES: the parentheses around the comparison are load-bearing -- an unparenthesized | ||
| // `>` inside a template parameter list closes the list instead of comparing (same | ||
| // reason `(kNumNotifyWarps > 0)` above is wrapped) | ||
| int kNumGeomParts = kMinTokensPerPart <= 1 ? kNumBudgetParts | ||
|
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. The geometry clamp is based on
Author
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. Good catch — the clamp keys off Doing it at runtime is more invasive than Happy to do that as a follow-up — kept out of this PR because the change surface (loops + sync + tail-skip) is much larger and warrants its own testing. Would you rather we merge this once the other comments are settled, or hold it and land runtime + compile-time together? |
||
| : ((kNumMaxTokensPerChannel / kMinTokensPerPart > 1) | ||
| ? kNumMaxTokensPerChannel / kMinTokensPerPart : 1), | ||
| int kNumParts = kNumBudgetParts < kNumGeomParts ? kNumBudgetParts : kNumGeomParts, | ||
| int kPartSize = math::constexpr_ceil_div(kNumMaxTokensPerChannel, kNumParts), | ||
| int kBatchSize = kPartSize, | ||
| int kNumSubParts = kNumSubPartsDefault < kBatchSize ? kNumSubPartsDefault : kBatchSize, | ||
|
|
||
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.
This should go to readme's env var list and clarify its default
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.
Done in the amended head — added
EP_MIN_TOKENS_PER_PART(default15,1disables the clamp) to the General list.