Skip to content

perf(scheduler): stop deep-copying whole Pods into the usage snapshot - #2978

Open
togettoyou wants to merge 1 commit into
Project-HAMi:masterfrom
togettoyou:perf/scheduler-pod-snapshot
Open

perf(scheduler): stop deep-copying whole Pods into the usage snapshot#2978
togettoyou wants to merge 1 commit into
Project-HAMi:masterfrom
togettoyou:perf/scheduler-pod-snapshot

Conversation

@togettoyou

@togettoyou togettoyou commented Sep 7, 2026

Copy link
Copy Markdown

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

The HAMi scheduler deep-copies the entire Pod cache on every Filter call, even though it only ever reads annotations and identity fields off those Pods. This PR replaces the whole-Pod deep copy with pointer sharing, while still copying the fields the manager owns.

getNodesUsage calls ListPodsInfo() once per Filter. ListPodsInfo calls PodInfo.DeepCopy() for every cached GPU Pod, and that includes corev1.Pod.DeepCopy() — a full copy of the PodSpec, containers, env, volumes, status, and the managedFields/FieldsV1 the API server generates.

getNodesUsage then attaches each *PodInfo to the matching device's DeviceUsage.PodInfos. When scoring runs per candidate node, scoreNode deep-copies the NodeUsage again — once for the app containers and once more per non-sidecar init container — and the old DeviceUsage.DeepCopy deep-copied every attached PodInfo, whole Pod included, a second time.

So one scheduling attempt performs roughly:

cached GPU pods × (1 + candidate nodes × (1 + non-sidecar init containers))

whole-Pod copies. The product grows with both cluster size and GPU Pod count. Nothing on the scheduling path mutates those Pods.

The change

  1. PodInfo.DeepCopyPodInfo.Snapshot: still deep-copies Devices (PodDevices) and the scalar fields the manager owns and rewrites in place, but shares the Pod pointer.
  2. DeviceUsage.DeepCopy: slices.Clone(d.PodInfos) replaces the per-element PodInfo.DeepCopy(). The copy gets its own slice header and shares the read-only *PodInfo entries.

Net production change is those two spots, about 10 lines.

Performance validation

Environment: Apple M1 / 16 GiB / macOS 25.5.0 / Go 1.27.1 darwin/arm64

KWOK + fake-gpu-operator

  • 128 nodes × 8 fake H100 = 1,024 GPUs, 10 shared slots each
  • 4,096 resident Pods (4 per GPU), loaded into the HAMi cache through the real API server and informers
  • 500 new Pods per round, 8 concurrent workers, each requesting 1 shared slot / 1,024 MiB / 10% cores
  • GOMAXPROCS=4 for both HAMi and kube-scheduler; HAMi with kube-qps=200, kube-burst=400, spread policy; kube-scheduler with percentageOfNodesToScore: 100
  • New Pods traverse the full path: API server → kube-scheduler → HAMi Filter/Score → HAMi Bind → API server → KWOK Running
  • Six interleaved rounds: before-1 → after-1 → before-2 → after-2 → before-3 → after-3. Each round deletes its own Pods and restarts HAMi; resident Pods and nodes stay put.

Steady-state results (3 rounds each, medians):

Metric before aa6f39d after (this PR) Change
Wall time for 500 Pods 34.89 s 20.26 s −41.9%
Successful throughput 14.33 pod/s 24.69 pod/s +72.2%
End-to-end P50 545.5 ms 343.4 ms −37.1%
End-to-end P95 676.8 ms 379.1 ms −44.0%
End-to-end P99 862.4 ms 396.8 ms −54.0%
Process allocation / new Pod 124.29 MiB 9.49 MiB −92.4%
Objects allocated (500-Pod window) 284,416,706 50,336,234 −82.3%
GC cycles 356 65 −81.7%
kube-scheduler scheduling-algorithm mean 69.60 ms 40.18 ms −42.3%

pprof:

before aa6f39d after (this PR)
Allocation alloc_space
60.84 GiB → 4.59 GiB
image image
CPU 60 s window
68.69 → 10.03 CPU·s
image image

benchmark:

BenchmarkListPodsInfo (pkg/device) sec/op B/op allocs/op
pods=100 195.6 µs → 31.4 µs (−83.96%) 537.6 KiB → 67.3 KiB (−87.48%) 1,902 → 802 (−57.83%)
pods=1000 2.019 ms → 310.8 µs (−84.61%) 5,375 KiB → 672 KiB (−87.50%) 19,003 → 8,003 (−57.89%)
pods=5000 13.040 ms → 1.901 ms (−85.42%) 26.25 MiB → 3.28 MiB (−87.50%) 95,003 → 40,003 (−57.89%)
BenchmarkScoreNode* (pkg/scheduler) sec/op B/op allocs/op
ScoreNodeOccupied podsPerGPU=0 (control) 10.36 µs → 10.49 µs (~, p=0.721) 11.00 KiB → 11.00 KiB (~) 154 → 154 (~)
ScoreNodeOccupied podsPerGPU=2 35.74 µs → 10.61 µs (−70.31%) 87.47 KiB → 11.31 KiB (−87.07%) 368 → 175 (−52.45%)
ScoreNodeOccupied podsPerGPU=4 61.08 µs → 11.28 µs (−81.53%) 163.85 KiB → 11.56 KiB (−92.94%) 562 → 177 (−68.51%)
ScoreNode initContainers=0/1/4 (control, idle devices) ~ (p=0.743 / 0.314 / 0.328) ~ ~ (identical per sample)

AI assistance disclosure

This PR was written with assistance from Claude Code, covering the code change, the benchmarks, and the setup and execution of the performance validation experiments. I have reviewed and understood every change line by line, and independently verified the correctness argument (the ownership boundary around the shared Pod and the read-only nature of each consumer).

Summary by CodeRabbit

  • Behavior Changes

    • Pod information retrieval now returns lightweight snapshots, sharing read-only pod data while keeping manager-owned fields independent.
    • Device usage copies now duplicate the pod information list while sharing its read-only entries.
  • Performance

    • Reduced copying overhead for pod and device usage information.
  • Tests

    • Added benchmarks covering pod snapshot generation and scoring nodes with existing workloads.
    • Updated tests to verify snapshot isolation and shared read-only data semantics.

@hami-robot

hami-robot Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: togettoyou
Once this PR has been reviewed and has the lgtm label, please assign shouren for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 4e638869-b4ca-41b4-b421-617a4c66e491

📥 Commits

Reviewing files that changed from the base of the PR and between 1092ba2 and 213bda5.

📒 Files selected for processing (6)
  • pkg/device/devices.go
  • pkg/device/devices_test.go
  • pkg/device/pod_test.go
  • pkg/device/pods.go
  • pkg/device/pods_bench_test.go
  • pkg/scheduler/score_bench_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

PodInfo now exposes snapshot semantics: manager-owned fields are copied, while the read-only Pod pointer is shared. Device usage copies clone only the PodInfos slice. Tests and benchmarks cover these semantics and occupied-node scoring.

Changes

Pod snapshot semantics

Layer / File(s) Summary
Snapshot API and retrieval paths
pkg/device/pods.go, pkg/device/devices.go, pkg/device/pod_test.go, pkg/device/devices_test.go
PodInfo.Snapshot() replaces DeepCopy(). It copies manager-owned fields, shares the Pod pointer, and is used by pod retrieval APIs. DeviceUsage.DeepCopy now clones only the PodInfos slice. Tests verify pointer sharing and manager-state isolation.
Snapshot and occupied-node benchmarks
pkg/device/pods_bench_test.go, pkg/scheduler/score_bench_test.go
Adds benchmarks for listing cached pod snapshots and scoring nodes with resident pods. Benchmark helpers create populated pod and device state. Klog output is suppressed during benchmark runs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 213bd

This change reduces scheduler allocation overhead by sharing read-only Pod objects while preserving independent device-accounting snapshots. The updated retrieval and copy semantics are covered, with no concrete current-head merge-blocking risk identified.

Suggested labels: enhancement

Poem

I’m a rabbit with snapshots tucked under my ear
Shared pods stay still, while device counts steer
Slices can change without breaking the view
Benchmarks hop through a thousand pods too
Occupied nodes now get measured anew

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main scheduler performance change: usage snapshots no longer deep-copy complete Pods.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@togettoyou
togettoyou force-pushed the perf/scheduler-pod-snapshot branch from 213bda5 to 5245cd0 Compare September 7, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant