Skip to content

Add configurable allocation policy (packed/distributed) for replicated and MIG resources - #1621

Merged
tariq1890 merged 1 commit into
NVIDIA:mainfrom
wkd-woo:feature/packed-allocation-policy
Jul 21, 2026
Merged

Add configurable allocation policy (packed/distributed) for replicated and MIG resources#1621
tariq1890 merged 1 commit into
NVIDIA:mainfrom
wkd-woo:feature/packed-allocation-policy

Conversation

@wkd-woo

@wkd-woo wkd-woo commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add --allocation-policy flag (ALLOCATION_POLICY env) with distributed (default) and packed options
  • packed mode bin-packs replicated/MIG devices onto fewest physical GPUs, freeing up remaining GPUs for full-GPU workloads
  • Default behavior (distributed) is unchanged — no breaking changes

Motivation

The current distributedAlloc was designed for time-slicing, where distributing replicas across physical GPUs avoids compute contention. However, MIG devices also fall into this code path simply because AlignedAllocationSupported() returns false for them — not because distributed allocation is the right strategy.

MIG instances are hardware-isolated partitions with dedicated SMs and memory. Packing them onto fewer physical GPUs has no performance penalty, and frees up remaining GPUs for full-GPU workloads:

┌─────────────────────────────────────────────────────────────────────┐
│                Distributed Allocation (current default)              │
├─────────────────────────────────────────────────────────────────────┤
│                                                                     │
│  GPU 0            GPU 1            GPU 2            GPU 3           │
│  ┌──────────┐    ┌──────────┐    ┌──────────┐    ┌──────────┐      │
│  │ ████░░░░ │    │ ████░░░░ │    │ ████░░░░ │    │ ░░░░░░░░ │      │
│  │ MIG 1/5  │    │ MIG 1/5  │    │ MIG 1/5  │    │ MIG 0/5  │      │
│  └──────────┘    └──────────┘    └──────────┘    └──────────┘      │
│   ⚠ partial       ⚠ partial       ⚠ partial       ░ empty          │
│                                                                     │
│  → Full GPU request arrives: only 1 GPU available (GPU 3)           │
│                                                                     │
└─────────────────────────────────────────────────────────────────────┘

┌─────────────────────────────────────────────────────────────────────┐
│                    Packed Allocation (bin-packing)                   │
├─────────────────────────────────────────────────────────────────────┤
│                                                                     │
│  GPU 0            GPU 1            GPU 2            GPU 3           │
│  ┌──────────┐    ┌──────────┐    ┌──────────┐    ┌──────────┐      │
│  │ ████████ │    │ ░░░░░░░░ │    │ ░░░░░░░░ │    │ ░░░░░░░░ │      │
│  │ MIG 3/5  │    │ MIG 0/5  │    │ MIG 0/5  │    │ MIG 0/5  │      │
│  └──────────┘    └──────────┘    └──────────┘    └──────────┘      │
│   ✓ packed         ★ free          ★ free          ★ free           │
│                                                                     │
│  → Full GPU request arrives: 3 GPUs available (GPU 1, 2, 3)        │
│                                                                     │
└─────────────────────────────────────────────────────────────────────┘

Relates to #491

Design

getPreferredAllocation
├─ Full GPU (AlignedAllocationSupported && no annotations)
│   └─ alignedAlloc (topology-based, unaffected by this change)
│
└─ MIG / time-slicing / MPS
├─ allocationPolicy=packed  → packedAlloc (new)
└─ allocationPolicy=distributed (default) → distributedAlloc (existing, unmodified)

Key decisions:

  • distributedAlloc is completely untouchedpackedAlloc is a separate function following the existing alignedAlloc/distributedAlloc pattern
  • Full GPU nodes are unaffectedalignedAlloc is selected before allocationPolicy is ever checked, so setting packed on a full-GPU node has no effect
  • Flag applies uniformly — when packed is set, it applies to MIG, time-slicing, and MPS. Silently ignoring a user-set flag for specific device types would be inconsistent
  • Per-node config supported — works with existing config-manager + ConfigMap + node label (nvidia.com/device-plugin.config) mechanism via YAML config

Usage

CLI flag / Environment variable

--allocation-policy=packed
# or
ALLOCATION_POLICY=packed
Config file (per-node via ConfigMap + node label)

version: v1
flags:
  migStrategy: mixed
  plugin:
    allocationPolicy: packed

kubectl label node mig-node nvidia.com/device-plugin.config=mig-packed

Test plan

  • TestDistributedAlloc (6 cases) — existing distributed behavior regression
  • TestDistributedAllocIsDefault — default consistency verified over 10 iterations
  • TestPackedAlloc (6 cases) — bin-packing: same GPU priority, overflow to next GPU
  • TestPackedVsDistributedContrast — two strategies produce different results on same input
  • TestFullGPUNodeIgnoresAllocationPolicy (3 sub-cases) — full GPU / MIG / replicated branch path verification
    All existing internal/rm/ tests pass unchanged

@copy-pr-bot

copy-pr-bot Bot commented Feb 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from 421e3c9 to 6686d1a Compare February 11, 2026 09:06
@ottowhite

ottowhite commented Feb 15, 2026

Copy link
Copy Markdown

Hi! Very cool idea, I actually really want a very similar custom allocation policy for replicated (non-MIG) resources, but for enabling bin-packing replicas with topology-awareness.

For example, this would could enable the allocation of 1.6 GPUs to a single model, with two tensor parallel ranks across 2 GPUs, 0.8 GPUs allocated to each tensor parallel tank, and both within a single NVLink domain (see my pretty diagram for an example). This would avoid excessive communication overhead during matrix multiplications across both tensor parallel model ranks, while leaving the 0.4 GPUs left over to be allocated to a smaller model, decreasing latency and increasing throughput in (surprisingly common) multi-model deployment scenarios including RAG with LLMs + Embedding models.

image

I believe the implementation of this policy is essentially a composite between what you've implemented, and the alignedAlloc code path which uses go-gpuallocator (gaining topology-awareness through NVML). At present, any of the replicated resource code paths discard topology awareness in favour of evenly load balancing GPU fractions with the distributed allocation policy, which prevents the use-case that I'm after.

Would be great if you're interested in discussing this more. I wonder if it's possible to slip it in with this PR? Also happy to contribute if that helps.

Comment thread internal/rm/allocate.go Outdated
jid := AnnotatedID(candidates[j]).GetID()
idiff := replicas[iid].total - replicas[iid].available
jdiff := replicas[jid].total - replicas[jid].available
return idiff > jdiff

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the only change in this function and distributedAlloc the comparator between idiff and jdiff? Can we just pass in an enum and then have a condition on the enum to dictate this behavior? There is a lot of code duplication at the moment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

The duplication is intentional — I wanted distributedAlloc to stay completely untouched so reviewers can verify there's zero behavioral change just by looking at the diff.

On merging them: the two functions share the same structure because packed is just the inverse of distributed (flipped comparator). But as you mentioned with topology-aware bin-packing, a future policy would need NVML topology data and different sorting logic entirely — so I'd rather keep them separate now than merge and split again later.

That said, happy to extract the common setup (candidate filtering + replica counting) into a shared helper if that feels cleaner.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem :) to be honest I think it would actually be easier to review/maintain if you generalised the allocation function slightly.

It could be generalised to fractionalAlloc, taking an enum that allows distributedFractionPlacement or packedFractionPlacement policies. Then internally you just pick the comparator based on the policy.

What you suggested with pulling out the common helpers such as candidate filtering and replica counting is probably even better than the generalisation I suggested, as it enables the policies to be kept completely separate, and is more extensible in that it enables easier implementation of new policies again such as the topology-aware one.

When I was reviewing the code, I pulled up both distributedAlloc and packedAlloc side by side and was cross-checking for a while before I saw the flipped comparator. I expect other reviewers and contributors to go through a similar process. This is just my two cents though.

You've also improved the state of testing so that's an additional signal that this is functionally equivalent to what there was before.

In terms of the different topology-aware placement policy, I think it makes sense to keep it separate for now. But cool that you're opening this up and making it more configurable! I may build off of this myself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ottowhite I've extracted prepareCandidates in the second commit.
Each policy function now only has its sorting logic, so the difference should be immediately visible

@ottowhite ottowhite Feb 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking better! There's still quite a bit of duplication across both of the Alloc functions. What is common is that they're both greedy allocation algorithms (repeatedly evaluating and selecting the next best option, without looking further ahead). Could you extract this into another helper greedyDeviceAlloc or something similar that makes the difference between these two functions even clearer? Could pass in a candidate comparator. It will also be much clearer to extend with other greedy allocation policies. For example the one that I spoke about is another greedy allocation algorithm that happens to use topology-awareness in it's comparator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should have a map of comparator functions to choose from. And a helper function that keeps the common portion in one place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 4afcd2e :) distributedAlloc/packedAlloc are gone — there is now a single greedyAlloc that takes a comparator, plus an allocationComparators map keyed by policy. The difference between the two policies ends up being just < vs > in the map. Unknown policies fall back to distributed (covered by a new test), and the existing tests pass unchanged.

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch 2 times, most recently from 8656ef3 to 2839bfc Compare February 17, 2026 12:22
@ottowhite

ottowhite commented Feb 17, 2026

Copy link
Copy Markdown

Also, is there a reason why the alignedAlloc happens to be in nvml_manager.go? It seems like just another allocation policy that would be more suited to be in allocate.go rather than there. I know this is not due to your code change but moving it there could improve the organisation of the code, and ease to extend with different allocation policies.

@wkd-woo

wkd-woo commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

Thank @ottowhite for your review :)
On moving alignedAlloc — agreed it could improve organization, but I'd rather keep this PR focused on the new feature and not restructure existing code!

I'd like to hear from a maintainer at this point to see if the current direction works.

@ottowhite

Copy link
Copy Markdown

Fair enough! Though I think those refactors would be a minor improvement for organisation/extensibility, I think the testing is strong and implementation is solid.

LGTM!

Will be great to hear what the maintainers have to say. Also really keen on this feature and the more extensible policy opportunities that it opens up.

@ottowhite

ottowhite commented Feb 19, 2026

Copy link
Copy Markdown

Hey! By the way we're running your packedAlloc allocation policy and it's working for multi-GPU deployments. We will imminently be forking and building off it it.

@wkd-woo

wkd-woo commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @elezar, just checking in to see if there's anything else needed from my side to move this forward. I'm happy to address any feedback :)

@wkd-woo

wkd-woo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi @rajatchopra, could you take a look at this PR when you have a chance?
I'd appreciate your feedback.

@rajatchopra rajatchopra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Pending:

  • rebase after #1788 merges
  • squash commits

@tariq1890

Copy link
Copy Markdown
Contributor

@wkd-woo Please rebase your PR on top of the latest main.

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from 4afcd2e to 59665d8 Compare July 1, 2026 03:12
@wkd-woo

wkd-woo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@rajatchopra @tariq1890 Rebased onto latest main (now includes #1788) and squashed into a single commit as requested.

Heads-up — this was not a clean rebase, so this needs another look before merge. #1788 landed the day after your /lgtm and it touches the exact same allocation path this PR refactors, so I had to reconcile them by hand:

To keep both, I folded the tie-break into greedyAlloc as a secondary sort key applied only when the comparator ranks two GPUs equally:

 sort.Slice(candidates, func(i, j int) bool {
     iid := AnnotatedID(candidates[i]).GetID()
     jid := AnnotatedID(candidates[j]).GetID()
-    return preferred(replicas[iid], replicas[jid])
+    ri, rj := replicas[iid], replicas[jid]
+    if preferred(ri, rj) {
+        return true
+    }
+    if preferred(rj, ri) {
+        return false
+    }
+    // comparator ranks the two GPUs equally -> prefer the physical
+    // device touched least during this allocation
+    return pickedFrom[iid] < pickedFrom[jid]
 })

This is the only logic change beyond the code you approved. It's required rather than optional: without it, #1788's regression test now fails on this branch (both replicas stack onto GPU-0); with it, the distributed policy keeps spreading. It's a no-op for packed, whose comparator dominates once packing starts. All internal/rm tests pass (TestDistributedAlloc, TestPackedAlloc, TestDistributedAllocIsDefault, TestPackedVsDistributedContrast, TestFullGPUNodeIgnoresAllocationPolicy, and #1788's TestDistributedAlloc_PartiallyAllocated_...).

PTAL when you get a chance.

@wkd-woo

wkd-woo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

cc @tariq1890 — flagging for the merge: this was rebased onto main after #1788 with one small logic change beyond @rajatchopra's /lgtm (folded #1788's distributed-allocation tie-break into this PR's new greedyAlloc — details and diff in the comment above). It's a no-op for packed, and all internal/rm tests pass including #1788's regression test. Should be ready to merge once CI is green — thanks!

@rajatchopra

rajatchopra commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@wkd-woo >> To keep both, I folded the tie-break into greedyAlloc as a secondary sort key applied only when the comparator ranks two GPUs equally
That is good work resolving the conflicts. Thanks. Needs another benign rebase though.

@rajatchopra rajatchopra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from 59665d8 to b2c3adf Compare July 20, 2026 08:09
@wkd-woo

wkd-woo commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@rajatchopra Rebased onto latest main. Clean, no conflicts, and all tests pass. Thanks!

cc. @tariq1890

Comment thread cmd/nvidia-device-plugin/main.go Outdated
case spec.AllocationPolicyDistributed:
case spec.AllocationPolicyPacked:
default:
return fmt.Errorf("invalid --allocation-policy option: %v", *config.Flags.Plugin.AllocationPolicy)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("invalid --allocation-policy option: %v", *config.Flags.Plugin.AllocationPolicy)
return fmt.Errorf("invalid --allocation-policy option: %s", *config.Flags.Plugin.AllocationPolicy)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Will Apply.

Comment thread internal/rm/allocate.go Outdated
// Get the set of candidate devices as the difference between available and required.
// replicaCount tracks the total and available replica counts for a physical GPU.
type replicaCount struct {
total, available int

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
total, available int
total int
available int

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Will Apply.

Comment thread cmd/nvidia-device-plugin/main.go Outdated
Name: "allocation-policy",
Value: spec.AllocationPolicyDistributed,
Usage: "the allocation policy for replicated and MIG resources:\n\t\t[distributed | packed]",
EnvVars: []string{"ALLOCATION_POLICY"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we come up with a more specific name here? The name ALLOCATION_POLICY does not lend itself to an allocation policy used for replicated and MIG devices.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about SHARED_DEVICES_ALLOCATION_POLICY?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Will rename to SHARED_DEVICES_ALLOCATION_POLICY / --shared-devices-allocation-policy, and update the config field to match.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you !

Comment thread internal/rm/allocate.go Outdated
Comment on lines +123 to +127
if preferred(ri, rj) {
return true
}
if preferred(rj, ri) {
return false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work?

Suggested change
if preferred(ri, rj) {
return true
}
if preferred(rj, ri) {
return false
if ri.allocated() != rj.allocated() {
return preferred(ri, rj)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that works. Both comparators only order by allocated(), so it comes out the same. I went with the two-way check to avoid assuming what the comparator compares, but with just these two policies that's probably overkill.
Will apply it, and add a small test on the comparators to cover that assumption.

@tariq1890

Copy link
Copy Markdown
Contributor

@wkd-woo Thank you very much for your patience with this PR and I appreciate all the work you've put in. Just a few more comments, but we are definitely looking to merge this and ensure that this lands in the next release of the device-plugin.

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from b2c3adf to b8c575e Compare July 21, 2026 02:41
@wkd-woo

wkd-woo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@tariq1890 Applied all four in b8c575ef, tests pass 🙏

@tariq1890

Copy link
Copy Markdown
Contributor

Can you squash your commit history?

@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from 14e9131 to 196c7c0 Compare July 21, 2026 05:25
@wkd-woo

wkd-woo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@tariq1890 Squashed !

@tariq1890 tariq1890 added this to the v0.20.0 milestone Jul 21, 2026
@tariq1890

Copy link
Copy Markdown
Contributor

/ok to test 196c7c0

@tariq1890

tariq1890 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@wkd-woo. Your commit is not signed. See here for context.

…d and MIG resources

The existing allocation spreads replicated devices evenly across
physical GPUs (distributed), which was designed for time-slicing where
workloads compete for shared compute. MIG instances, however, are
hardware-isolated and do not suffer from contention when packed onto the
same GPU.

This adds a --shared-devices-allocation-policy flag
(env: SHARED_DEVICES_ALLOCATION_POLICY) with two
options: "distributed" (default, preserving current behavior) and
"packed" (bin-packing onto the fewest physical GPUs). The packed policy
frees up entire GPUs for full-GPU workloads in mixed clusters. The flag
applies uniformly to all non-aligned allocation paths (MIG,
time-slicing, MPS) and can be configured per-node via ConfigMap and the
nvidia.com/device-plugin.config node label. Full-GPU nodes are
unaffected: alignedAlloc is selected before the policy is ever consulted.

Both policies share a single greedyAlloc helper that differs only in a
replicaComparator, selected from an allocationComparators map keyed by
policy (comparatorForPolicy falls back to distributed for unknown
values). This keeps the strategies expressed as a flipped comparator and
lets future policies be added as new map entries without duplicating the
selection loop. When the comparator ranks two physical GPUs equally, a
per-allocation pickedFrom tie-break rotates to the least-touched sibling,
preserving distribution across physical GPUs.

Relates to NVIDIA#491

Signed-off-by: wkd-woo <wkdwoos@gmail.com>
@wkd-woo
wkd-woo force-pushed the feature/packed-allocation-policy branch from 196c7c0 to 891d77f Compare July 21, 2026 17:13
@wkd-woo

wkd-woo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@tariq1890 Signed the commit. Thank you for your patience.

@tariq1890

Copy link
Copy Markdown
Contributor

/ok to test 891d77f

@tariq1890
tariq1890 merged commit 88a79d7 into NVIDIA:main Jul 21, 2026
38 of 41 checks passed
jonathan-meiri pushed a commit to jonathan-meiri/k8s-device-plugin that referenced this pull request Jul 22, 2026
Follow-up on top of NVIDIA#1621, which introduced the shared greedyAlloc loop
with a pluggable replicaComparator (distributed vs packed). The loop
still sorts the full candidate slice inside the allocation loop, paying
O(n log n) per iteration for n iterations and giving O(n² log n) overall.

Since all annotated replicas from the same underlying physical device
share the same sort key, sorting at the replica granularity is wasted
work — only m (the number of distinct physical devices contributing
candidates) needs to be reordered.

Refactor greedyAlloc to bucket candidates by their underlying physical
device into a small gpuAllocState per device, holding a shared
*replicaCount, the pickedFrom counter, and the remaining candidate IDs.
A gpuPriorityQueue defers to the caller-supplied replicaComparator on
allocated() for primary ordering and to pickedFrom for the tie-break
(unchanged semantics). Each iteration pops the best device, takes one
of its remaining replicas, updates counters, and pushes it back if any
remain.

Total cost drops to O(n log m). Both allocation policies (distributed
and packed) benefit; no behavior change — the existing test suite
(TestDistributedAlloc, TestPackedAlloc, TestPackedVsDistributedContrast,
TestDistributedAlloc_PartiallyAllocated_DistributesAcrossDistinctGPUs,
etc.) passes unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
jonathan-meiri pushed a commit to jonathan-meiri/k8s-device-plugin that referenced this pull request Jul 26, 2026
Follow-up on top of NVIDIA#1621, which introduced the shared greedyAlloc loop
with a pluggable replicaComparator (distributed vs packed). The loop
still sorts the full candidate slice inside the allocation loop, paying
O(n log n) per iteration for n iterations and giving O(n² log n) overall.

Since all annotated replicas from the same underlying physical device
share the same sort key, sorting at the replica granularity is wasted
work — only m (the number of distinct physical devices contributing
candidates) needs to be reordered.

Refactor greedyAlloc to bucket candidates by their underlying physical
device into a small gpuAllocState per device, holding a shared
*replicaCount, the pickedFrom counter, and the remaining candidate IDs.
A gpuPriorityQueue defers to the caller-supplied replicaComparator on
allocated() for primary ordering and to pickedFrom for the tie-break
(unchanged semantics). Each iteration pops the best device, takes one
of its remaining replicas, updates counters, and pushes it back if any
remain.

Total cost drops to O(n log m). Both allocation policies (distributed
and packed) benefit; no behavior change — the existing test suite
(TestDistributedAlloc, TestPackedAlloc, TestPackedVsDistributedContrast,
TestDistributedAlloc_PartiallyAllocated_DistributesAcrossDistinctGPUs,
etc.) passes unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
jonathan-meiri pushed a commit to jonathan-meiri/k8s-device-plugin that referenced this pull request Jul 28, 2026
Follow-up on top of NVIDIA#1621, which introduced the shared greedyAlloc loop
with a pluggable replicaComparator (distributed vs packed). The loop
still sorts the full candidate slice inside the allocation loop, paying
O(n log n) per iteration for n iterations and giving O(n² log n) overall.

Since all annotated replicas from the same underlying physical device
share the same sort key, sorting at the replica granularity is wasted
work — only m (the number of distinct physical devices contributing
candidates) needs to be reordered.

Refactor greedyAlloc to bucket candidates by their underlying physical
device into a small gpuAllocState per device, holding a shared
*replicaCount, the pickedFrom counter, and the remaining candidate IDs.
A gpuPriorityQueue defers to the caller-supplied replicaComparator on
allocated() for primary ordering and to pickedFrom for the tie-break
(unchanged semantics). Each iteration pops the best device, takes one
of its remaining replicas, updates counters, and pushes it back if any
remain.

Total cost drops to O(n log m). Both allocation policies (distributed
and packed) benefit; no behavior change — the existing test suite
(TestDistributedAlloc, TestPackedAlloc, TestPackedVsDistributedContrast,
TestDistributedAlloc_PartiallyAllocated_DistributesAcrossDistinctGPUs,
etc.) passes unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
jonathan-meiri pushed a commit to jonathan-meiri/k8s-device-plugin that referenced this pull request Aug 2, 2026
Follow-up on top of NVIDIA#1621, which introduced the shared greedyAlloc loop
with a pluggable replicaComparator (distributed vs packed). The loop
still sorts the full candidate slice inside the allocation loop, paying
O(n log n) per iteration for n iterations and giving O(n² log n) overall.

Since all annotated replicas from the same underlying physical device
share the same sort key, sorting at the replica granularity is wasted
work — only m (the number of distinct physical devices contributing
candidates) needs to be reordered.

Refactor greedyAlloc to bucket candidates by their underlying physical
device into a small gpuAllocState per device, holding a shared
*replicaCount, the pickedFrom counter, and the remaining candidate IDs.
A gpuPriorityQueue defers to the caller-supplied replicaComparator on
allocated() for primary ordering and to pickedFrom for the tie-break
(unchanged semantics). Each iteration pops the best device, takes one
of its remaining replicas, updates counters, and pushes it back if any
remain.

Total cost drops to O(n log m). Both allocation policies (distributed
and packed) benefit; no behavior change — the existing test suite
(TestDistributedAlloc, TestPackedAlloc, TestPackedVsDistributedContrast,
TestDistributedAlloc_PartiallyAllocated_DistributesAcrossDistinctGPUs,
etc.) passes unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants