Repository navigation
Fix Task Manager CPU samples - #3588
Merged
Merged
Conversation
Add a behavioral unit test that launches a CPU-bound child process, captures the top snapshot twice, and expects the second sample to report non-zero CPU for that process. The current implementation reads the zero-valued kinfo p_pctcpu field, so this locks the regression at the sampler boundary shared by Task Manager and cmux top. Constraint: Local tests are intentionally not run in this repo; CI owns test execution. Rejected: UI-only Task Manager assertion | the same zero reaches cmux top, so the sampler boundary is the correct behavioral seam. Confidence: high Scope-risk: narrow Tested: Not run locally per repository policy. Not-tested: CI has not run this failing-test-only commit yet.
The top sampler was reading kinfo_proc.p_pctcpu, which is zero for the processes exposed through the Task Manager/cmux top pipeline on current macOS. Keep the existing rollup and presentation code, but make CmuxTopProcessSnapshot own CPU sampling by recording PROC_PIDTASKINFO total_user+total_system counters per process identity and calculating a rate against monotonic wall-clock nanoseconds on the next capture. Constraint: Task Manager and cmux top share the same system.top payload; fix the leaf sampler instead of patching UI rows or CLI formatting. Rejected: Poll p_pctcpu again after a delay | the API source is stale/zero for this use path and would keep the first sample wrong. Rejected: Key previous samples by PID only | PID reuse would mix unrelated processes, so the existing pid+start-time key is reused. Confidence: high Scope-risk: narrow Directive: Keep cpu_percent sourced from CmuxTopProcessSnapshot so Task Manager and cmux top cannot diverge. Tested: ./scripts/reload.sh --tag issue-3583-task-manager-cpu-zero --launch built and launched before the Rosetta wall-clock correction; tagged cmux top then showed non-zero CPU for a busy yes process, revealing the final unit adjustment. Not-tested: Local unit tests were not run per repository policy; CI must run the new regression test.
Merged origin/main before opening the PR so CI runs against the current workflow set. The merge only brings in the latest CI auto-approve workflow/script additions and does not change the CPU sampling implementation. Constraint: iterate-pr workflow requires syncing the PR branch with the base branch before CI iteration. Confidence: high Scope-risk: narrow Directive: Treat this as a base-branch sync only; CPU behavior lives in the two preceding commits. Tested: Git merge completed without conflicts. Not-tested: Local tests not run per repository policy.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds sampling-based per-process CPU tracking and conversion helpers, integrates sampling into top snapshot construction and payloads, replaces scope-cache locking with an unfair lock, adds process-introspection helpers (WKWebView PID), includes unit tests for CPU sampling, and updates the Xcode project to include new sources and tests. ChangesCPU tracking + process details integration
Sequence DiagramsequenceDiagram
actor Main as Main Thread
participant Snapshot as CmuxTopSnapshot
participant Clock as System Clock
participant ProcInfo as proc_pidtaskinfo
participant Tracker as CmuxTopProcessCPUTracker
Main->>Snapshot: allProcesses(includeProcessDetails)
Snapshot->>Clock: cpuSampleClockNanoseconds()
Clock-->>Snapshot: now (ns)
loop per-process
Snapshot->>ProcInfo: proc_pidtaskinfo(pid)
ProcInfo-->>Snapshot: task info (user/system ticks)
Snapshot->>Tracker: record/update sample for scope key
end
Snapshot->>Tracker: cpuPercentages(activeKeys, sampledAt)
Tracker->>Tracker: compute tick delta → ns, calc per-key %
Tracker-->>Snapshot: per-scope cpu% map
Snapshot->>Snapshot: apply cpu% to CmuxTopProcessInfo
Snapshot-->>Main: updated process list with cpuPercent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryReplaces the always-zero
Confidence Score: 5/5Safe to merge — the CPU sampling path is self-contained, all new state is guarded by a documented lock, and the fix is well-scoped to the zero-CPU regression. All new types carry explicit nonisolated annotations; the OSAllocatedUnfairLock usage is documented and constrained to arithmetic-only work inside the critical section; overflow sentinels are caught before any division; out-of-order capture protection is implemented correctly; and all prior review feedback has been fully addressed in this revision. No files require special attention. CmuxTopProcessCPUTracker.swift is the highest-risk new file but is small, well-commented, and the lock invariant is narrow and documented. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[allProcesses called] --> B[sysctl KERN_PROC_ALL]
B --> C[Build activeScopeKeys from all kinfo_proc]
C --> D[cpuSampleClockNanoseconds single CLOCK_UPTIME_RAW timestamp]
D --> E{For each process}
E --> F[proc_pidinfo PROC_PIDTASKINFO]
F -->|success| G[cpuSample: ticks + timestamp to currentCPUSamples map]
F -->|failure| H[cpuSampleKey = nil, memory = 0]
G --> E
H --> E
E -->|done| I[CmuxTopProcessCPUTracker.cpuPercentages one OSAllocatedUnfairLock transaction]
I --> J{For each key in currentSamples}
J --> K{existing sample newer?}
K -->|yes| L[skip preserve newer sample]
K -->|no| M{either sample is UInt64.max?}
M -->|yes| N[cpuPercent = 0]
M -->|no| O[delta ticks x mach_timebase ratio divided by wall ns x 100]
N --> P[state.samples update]
O --> P
P --> J
J -->|done| Q{sampledAt >= latestPrunedAt?}
Q -->|yes| R[prune state.samples to activeKeys]
Q -->|no| S[skip prune older capture]
R --> T[return percentages map]
S --> T
T --> U[Apply cpuPercent to each processRecord]
U --> V[pruneCMUXScopeCache activeKeys]
V --> W[Return CmuxTopProcessInfo array]
Reviews (8): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The CPU rate cache remains synchronous because it serves both the Task Manager refresh path and the sync system.top socket command, but the mutable history now has one small owner with lock-scoped reads and writes. Recording merges new samples before pruning stale ones so overlapping captures cannot discard newer state written by another caller. Constraint: CmuxTopProcessSnapshot.capture is synchronous for socket and UI callers Rejected: Convert capture to an actor API | would widen this bug fix across socket handlers and callers unrelated to the CPU-zero regression Confidence: high Scope-risk: narrow Directive: Keep proc/sysctl sampling outside the sample-store critical section Tested: git diff --check Not-tested: Local XCTest per repository policy
The task-manager and v2 system.top paths share CmuxTopProcessSnapshot.capture, and one caller is intentionally synchronous. The CPU history is now owned by a dedicated tracker that computes percentages and commits samples in one locked transaction, removing the previous read-then-record split that could race between overlapping captures. Process detail helpers moved out of CmuxTopSnapshot so the guard budget stays under the threshold without accepting new debt. Constraint: CmuxTopProcessSnapshot.capture still backs the synchronous v2 system.top socket path Rejected: Actor-owned CPU sample store | would require making capture async and either blocking the socket worker or widening socket command execution beyond this PR Confidence: high Scope-risk: narrow Directive: Keep proc/sysctl sampling outside the CPU tracker lock; only the sample history transaction belongs inside the owner Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check Not-tested: Full app build deferred until CI is green, then ./scripts/reload.sh --tag issue-3583-task-manager-cpu-zero --launch
Overlapping top captures can complete in a different order than they sampled. The CPU tracker now stores both sample history and the latest prune timestamp, so an older capture can update missing older samples but cannot evict entries from a newer completed capture. Constraint: Keep CmuxTopProcessSnapshot.capture synchronous for the v2 system.top socket path Rejected: Rely on sampledAtNanoseconds > during filter | hid the out-of-order invariant inside an impossible-looking branch Confidence: high Scope-risk: narrow Directive: Pruning must remain monotonic by sample time; do not let older captures evict newer sample history Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check Not-tested: Full app build deferred until CI is green, then ./scripts/reload.sh --tag issue-3583-task-manager-cpu-zero --launch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@GhosttyTabs.xcodeproj/project.pbxproj`:
- Line 244: The PBXBuildFile entry with ID C7A50C000000000000002 references a
nonexistent PBXFileReference ID C7A50C000000000001; update that build file's
fileRef to the actual file reference ID C7A50C000000000000001 (the
PBXFileReference created for CmuxTopProcessCPUTests.swift) so the
CmuxTopProcessCPUTests.swift build file is correctly wired into the project
(locate the PBXBuildFile entry for C7A50C000000000000002 and replace its fileRef
value).
In `@Sources/CmuxTopProcessCPUTracker.swift`:
- Around line 27-37: The code computes cpuPercent using storedSamples[key]
before checking whether the incoming sample is actually newer, which lets a
later-sampled-but-earlier-committed sample produce 0% and prevent the rightful
newer sample from being stored; fix by first reading storedSamples[key] into a
local (e.g., let existing = storedSamples[key]), perform the recency guard (if
let existing = existing, existing.sampledAtNanoseconds >
sample.sampledAtNanoseconds { continue }), then compute percentages[key] =
CmuxTopProcessSnapshot.cpuPercent(current: sample, previous: existing) and
finally assign storedSamples[key] = sample if the sample is newer, ensuring
cpuPercent uses the correct previous snapshot and updates happen in the correct
order.
In `@Sources/CmuxTopProcessDetails.swift`:
- Around line 32-37: The fixedString<T>(_:) helper uses String(cString:) which
can read past the passed rawBuffer if there is no NUL inside the fixed-width C
buffer; change it to scan the bytes returned by withUnsafeBytes(rawBuffer) /
chars (bound to CChar) for the first zero within rawBuffer.count, get the
slice/length up to that index (or rawBuffer.count if none), then construct a
Swift String from those bytes (or use String(bytes:encoding:)) and trim
whitespace; update references in fixedString, withUnsafeBytes, rawBuffer, chars,
and baseAddress to ensure you only decode up to that located NUL to avoid
out-of-bounds reads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdc6e803-f154-4558-8f6f-6ab610493c25
📒 Files selected for processing (5)
GhosttyTabs.xcodeproj/project.pbxprojSources/CmuxTopProcessCPUTracker.swiftSources/CmuxTopProcessDetails.swiftSources/CmuxTopSnapshot.swiftcmuxTests/CmuxTopProcessCPUTests.swift
Code review found one broken PBX file reference and two unsafe CPU sampling edges. The tracker now ignores stale overlapping samples before calculating rates, and fixed-width process strings decode only inside their known buffer bounds. Constraint: PR review requires every actionable CodeRabbit thread to be addressed before the final tagged reload Confidence: high Scope-risk: narrow Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: plutil -lint GhosttyTabs.xcodeproj/project.pbxproj Tested: git diff --check
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
May 6, 2026 07:34
Dismissed as obsolete after follow-up fixes. All three CodeRabbit inline threads were fixed in commit 5ca62d7, replied to, resolved, and CodeRabbit subsequently acknowledged the fixes; the latest CodeRabbit status check is passing.
Greptile flagged that the lock-owned CPU tracker state should carry the same explicit isolation intent as the CPU sample value. The state remains synchronously owned by OSAllocatedUnfairLock; this change documents that the value type itself is not MainActor-isolated. Constraint: Post-merge review feedback requested an annotation-only Swift fix Confidence: high Scope-risk: narrow Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check
The process snapshot path is not UI-owned: Task Manager samples it through a detached task and system.top samples it from a socket-worker path. Under MainActor default isolation, helper extensions and module globals still inherited ambient main-actor isolation even though their state is protected by process-snapshot locks. This marks the process snapshot value domain and helper extensions nonisolated, makes the CPU tracker Sendable conformance explicit, and moves the scope cache from NSLock plus a mutable global to an OSAllocatedUnfairLock-owned state value. That keeps the synchronous API intact while making the non-UI ownership boundary explicit to Swift 6 isolation checks. Constraint: Do not make system.top async; it is a synchronous socket-worker RPC path. Rejected: Convert capture to an actor | would force async plumbing through the sync socket response path and add actor hops around a tiny dictionary transaction. Rejected: Use nonisolated(unsafe) on mutable globals | OSAllocatedUnfairLock expresses the synchronization boundary directly. Confidence: high Scope-risk: narrow Directive: Keep proc/sysctl/WebKit process inspection outside lock critical sections; locks own cache/history dictionaries only. Tested: swiftc -default-isolation MainActor -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check Not-tested: xcodebuild and local test bundles, per repo policy and task instruction.
Greptile's final review noted that the two-pass CPU snapshot flow was correct but carried a fragile parallel-array join, and that the mach_timebase_info fallback deserved a second look. This collapses the join into a single tuple record list and makes timebase conversion fail closed to 0% when the system ratio cannot be loaded. Constraint: Preserve the existing synchronous capture API and shared system.top payload. Rejected: Leave the observations as comments | the tuple record shape is a smaller and more durable invariant than explaining parallel arrays. Rejected: Keep ratio fallback at 1 | that can report plausible but incorrect CPU if mach_timebase_info fails. Confidence: high Scope-risk: narrow Directive: Keep each process info paired with its CPU sample key until CPU percentages are applied; do not reintroduce parallel arrays for this path. Tested: swiftc -default-isolation MainActor -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check Not-tested: xcodebuild and local test bundles, per repo policy and task instruction.
Greptile's last review found that extracting process-detail helpers widened private snapshot construction utilities to module-internal access. The helper calls are only needed by the private processInfo path, so keep them private in the snapshot file and move the already-internal cmux scope parsing beside the scope cache that consumes it. Constraint: Do not touch YAML files while updating this PR Rejected: Leave helpers internal | preserves an avoidable API-surface expansion Confidence: high Scope-risk: narrow Directive: Keep low-level proc/task decoding helpers scoped to snapshot construction unless a real external caller appears Tested: swiftc -default-isolation MainActor -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/CmuxTopSnapshotScopeCache.swift (1)
33-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent PID-reuse misattribution on cache miss
On a miss, scope is fetched via
cmuxScope(for: pid)(PID-only) and then cached under a(pid + start time)key. If the PID is reused between sampling and this lookup, you can cache the wrong process scope under the old identity.Proposed fix
static func cachedCMUXScope( for pid: Int, cacheKey: CmuxTopProcessScopeCacheKey ) -> CmuxTopProcessScope? { if let cached = cmuxTopScopeCache.withLock({ cache in cache[cacheKey] }) { return cached.scope } + // Re-validate identity before resolving/caching by PID. + guard let current = kinfoProc(for: pid), + scopeCacheKey(from: current) == cacheKey else { + return nil + } + guard let scope = cmuxScope(for: pid) else { return nil } cmuxTopScopeCache.withLock { cache in cache[cacheKey] = CmuxTopProcessScopeCacheValue(scope: scope) } return scope }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/CmuxTopSnapshotScopeCache.swift` around lines 33 - 47, The cachedCMUXScope(for:cacheKey:) function fetches a scope with cmuxScope(for: pid) that may belong to a different process if the PID was recycled; to fix, after obtaining scope verify the process identity matches the cache key (e.g. compare scope.startTime or other unique identity field to cacheKey.startTime) and only insert into cmuxTopScopeCache via cmuxTopScopeCache.withLock { ... } when they match; if they don't match, return nil (and do not cache). Also ensure the initial cache read still returns only when cached.scope's identity matches cacheKey before returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/CmuxTopProcessDetails.swift`:
- Around line 5-14: Mark the pid(for:) helper as main-actor-isolated by adding
the `@MainActor` attribute to its declaration (i.e. change the signature of static
func pid(for webView: WKWebView) -> Int? to be `@MainActor` static func pid(for
webView: WKWebView) -> Int?) so all access to the main-thread-only WKWebView API
is constrained to the main actor; after changing the signature, update any
callers invoked from non-main contexts to await/dispatch to the main actor
(e.g., call from taskManagerTopPayload or other background code via Task {
`@MainActor` in ... } or await MainActor.run { ... }) so the function is never
invoked off the main thread.
---
Outside diff comments:
In `@Sources/CmuxTopSnapshotScopeCache.swift`:
- Around line 33-47: The cachedCMUXScope(for:cacheKey:) function fetches a scope
with cmuxScope(for: pid) that may belong to a different process if the PID was
recycled; to fix, after obtaining scope verify the process identity matches the
cache key (e.g. compare scope.startTime or other unique identity field to
cacheKey.startTime) and only insert into cmuxTopScopeCache via
cmuxTopScopeCache.withLock { ... } when they match; if they don't match, return
nil (and do not cache). Also ensure the initial cache read still returns only
when cached.scope's identity matches cacheKey before returning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41bf6767-1dfc-44f6-bc71-8d1ebb2bc1a6
📒 Files selected for processing (4)
GhosttyTabs.xcodeproj/project.pbxprojSources/CmuxTopProcessDetails.swiftSources/CmuxTopSnapshot.swiftSources/CmuxTopSnapshotScopeCache.swift
Review found that a PID-only procargs lookup could run after the sampled PID had been recycled, then cache the replacement process scope under the old pid+start-time identity. The cache miss path now resolves procargs only when the current kinfo_proc identity matches before and after the lookup, and the WebKit PID bridge is explicitly main-actor isolated. Constraint: Scope cache is intentionally synchronous because top snapshots are used by both async UI sampling and sync socket paths Rejected: Cache PID-only procargs results without revalidation | can misattribute CMUX scope after PID reuse Confidence: high Scope-risk: narrow Directive: Do not cache process-derived scope data unless the pid+start-time identity still matches the sampled key Tested: swiftc -default-isolation MainActor -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swift Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Tested: git diff --check
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
May 7, 2026 01:34
Stale CodeRabbit changes-requested review from fbe5eeb. The requested fixes were addressed in 7c71267/af82466a1: the WebKit PID helper is @mainactor and scope-cache misses revalidate pid+start-time identity before caching. Latest checks are green, CodeRabbit status is success, Greptile reviewed af82466 as safe, and all review threads are resolved.
This was referenced May 13, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3583.
This keeps Task Manager and
cmux topon the same sharedsystem.toppayload, but changes the leaf CPU sampler from the stalekinfo_proc.p_pctcpufield to a real rate calculation fromPROC_PIDTASKINFOcumulative CPU time.Root Cause
CmuxTopProcessSnapshot.processInfoinSources/CmuxTopSnapshot.swiftreturned:That field is zero for the process rows exposed by the Task Manager /
cmux toppath on current macOS, soCmuxTopProcessSnapshot.summary(for:)correctly summed zeros for every leaf, and every aggregate row also displayed0.0%.The regression was introduced with the original Task Manager/top implementation in
971372531fcc7141c5da87c5bdd724da1f5ee0b6(Add top snapshots and Task Manager window (#3290)), which madekinfo_proc.p_pctcputhe CPU source while memory already came fromproc_pidinfo(PROC_PIDTASKINFO).Fix
CmuxTopProcessSnapshot.summary(for:)reports non-zero CPU after a second capture.CmuxTopProcessCPUTrackeras the single owner for CPU sample history.pid+ process start time), not PID alone.Task Manager rows and CLI formatting continue to consume the same
cpu_percentpayload field; no UI redesign or new metric was added.Validation
./scripts/reload.sh --tag issue-3583-task-manager-cpu-zero --launchbuilt and launched the tagged app during validation.yes > /dev/null, taggedcmux top --processesshowed non-zero CPU for the active process before the final Rosetta wall-clock correction.PROC_PIDTASKINFOprobe confirmed the busy process reports live cumulative CPU deltas; the final implementation uses monotonic wall-clock nanoseconds to avoid Rosettamach_absolute_time()unit mismatch.Local unit tests were not run per repo policy; CI should run the new regression test.
Testing
swiftc -typecheck Sources/CmuxTopSnapshot.swift Sources/CmuxTopSnapshotScopeCache.swift Sources/CmuxTopProcessCPUTracker.swift Sources/CmuxTopProcessDetails.swiftpython3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsvplutil -lint GhosttyTabs.xcodeproj/project.pbxprojgit diff --checkxcodebuildor local app test bundles, per repo policy and this PR's build-safety instructions.Checklist
cmux top) #3583.cmux topon the sharedsystem.top/cpu_percentpayload path.xcodebuild; final validation uses the required tagged reload command.Note
Medium Risk
Updates low-level process sampling and caching/locking around
system.top, which could affect Task Manager/cmux topcorrectness or performance across macOS versions.Overview
Fixes Task Manager/
cmux topCPU% reporting by replacing the stalekinfo_proc.p_pctcpufield with a rate computed fromPROC_PIDTASKINFOcumulative CPU time deltas over a monotonic clock, while keeping the existingsystem.toppayload shape (cpu_percent).Adds
CmuxTopProcessCPUTrackerto own per-process CPU sample history (keyed by pid+start time), handle overlapping/out-of-order captures, and prune inactive keys safely. Also refactors CMUX scope caching to useOSAllocatedUnfairLockand moves the WebKit content-process PID helper intoCmuxTopProcessDetails.swift.Includes a new regression test suite (
CmuxTopProcessCPUTests) that validates non-zero CPU for a busy child process and zero CPU for overflow-sentinel samples.Reviewed by Cursor Bugbot for commit af82466. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fix CPU% in Task Manager and
cmux topby computing rates fromPROC_PIDTASKINFOdeltas on a monotonic clock with a locked per‑process history, and harden CMUX scope caching against PID reuse. Keeps the existingsystem.toppayload and returns 0% if the timebase ratio can’t be loaded.Bug Fixes
pti_total_user + pti_total_systemoverCLOCK_UPTIME_RAWns viamach_timebase_info; fall back to 0% if the ratio is unavailable.CmuxTopProcessCPUTrackerto compute+commit in one locked step, key by process identity (pid + start time), ignore stale overlapping samples, and prune only from the newest capture.KERN_PROCARGS2; only cache when the identity still matches.Refactors
nonisolated; replaceNSLock+global scope cache withOSAllocatedUnfairLock, move CMUX scope parsing next to the cache, keep process‑detail helpers private, and extract the WebKit PID helper toCmuxTopProcessDetails.swift.Written for commit af82466. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Tests