Skip to content

Conversation

@IvanKarpan
Copy link

@IvanKarpan IvanKarpan commented Nov 11, 2025

Problem

On systems like M4 Pro 12/16, when performance cores are fully utilized, the Details section incorrectly shows Performance Cores: 75% instead of 100%, even though the bar graph displays correctly.

This occurs when CPU IDs from IORegistry don't align with sequential logical CPU indices from host_processor_info. The buggy code used CPU IDs directly as array indices (usagePerCore[Int(c.id)]), causing out-of-bounds lookups that returned 0 for cores with non-sequential IDs (e.g., IDs 20, 21 when the array only has indices 0-11).

Issue: #2785

Solution

Replaced ID-based matching with positional matching to correctly map cores to usage data:

  • Changed from: usagePerCore[Int(c.id)] (uses CPU ID as index)
  • Changed to: usagePerCore[index] (uses position in cores array)

This ensures that cores[i] always maps to usagePerCore[i], regardless of what CPU IDs are stored in the core objects.

Changes

Core Fix

  • Extract calculation logic into testable public functions:
    • calculatePerformanceCoresUsage(cores:usagePerCore:)
    • calculateEfficiencyCoresUsage(cores:usagePerCore:)
  • Both functions now use positional matching via enumerated().compactMap
  • Add array count validation (cores.count == usagePerCore.count) before calculations
  • Update LoadReader to use the new functions

Testing

  • Add comprehensive test suite with 13 test cases:

Testing

All 13 tests pass, including:

  • ✅ Original bug scenario (75% → 100% fix)
  • ✅ Positional matching with non-sequential CPU IDs
  • ✅ Edge cases (empty arrays, mismatches, zero usage)
  • ✅ Mixed usage calculations

Impact

  • Fixes: Performance cores now correctly show 100% when fully utilized
  • Backward compatible: No API changes, only internal implementation improvements
  • Test coverage: Comprehensive test suite prevents regressions
  • Code quality: Extracted functions improve testability and maintainability

…tilized

replace CPU ID-based matching with positional matching in core usage calculations
extract calculation logic into testable public functions (calculatePerformanceCoresUsage, calculateEfficiencyCoresUsage)
add array count validation before performing calculations
add comprehensive test suite (13 tests) covering bug scenario, edge cases, and positional matching
update SwiftLint config: rename redundant_optional_initialization to implicit_optional_initialization
remove unnecessary SwiftLint disable comment in Charts.swift
fix RAM test: update test case PID to avoid conflicts
Fixes exelban#2785
@exelban
Copy link
Owner

exelban commented Nov 11, 2025

Hi. Please remove the comments, and please do not make any changes that are related to the problem.

revert update SwiftLint config: rename redundant_optional_initialization to implicit_optional_initialization
revert remove unnecessary SwiftLint disable comment in Charts.swift
revert fix RAM test: update test case PID to avoid conflicts
@IvanKarpan
Copy link
Author

IvanKarpan commented Nov 12, 2025

Removed all comments I introduced and reverted changes unrelated to #2785.

Do you want the other improvements (SwiftLint config deprecated warning fix, unnecessary SwiftLint disable comment, fix for RAM test) in a separate PR?

@exelban
Copy link
Owner

exelban commented Nov 12, 2025

thx, LGTM

@exelban
Copy link
Owner

exelban commented Nov 13, 2025

Hi, I’ve taken a closer look at the changes you proposed, and I have a question. Why are there so many modifications for such a small fix? Below is the version I received after reviewing this PR:

            let eCoresList: [Double] = cores.filter({ $0.type == .efficiency }).enumerated().compactMap { (i, c) -> Double? in
                if self.response.usagePerCore.indices.contains(Int(c.id)) {
                    return self.response.usagePerCore[Int(c.id)]
                }
                return i < usagePerCore.count ? usagePerCore[i] : 0
            }
            let pCoresList: [Double] = cores.filter({ $0.type == .performance }).enumerated().compactMap { (i, c) -> Double? in
                if self.response.usagePerCore.indices.contains(Int(c.id)) {
                    return self.response.usagePerCore[Int(c.id)]
                }
                return i < usagePerCore.count ? usagePerCore[i] : 0
            }

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.

2 participants