Dedup Linux CentralProcessor topology, frequency, and load-average logic - #3463
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (13)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2026-04-06T04:45:47.644ZApplied to files:
📚 Learning: 2026-07-06T19:25:59.225ZApplied to files:
🔇 Additional comments (4)
📝 WalkthroughWalkthrough
ChangesLinux CPU refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LinuxCentralProcessor
participant NativeBackend
participant ProcLoadavg
LinuxCentralProcessor->>NativeBackend: getloadavgNative(loadavg, nelem)
NativeBackend-->>LinuxCentralProcessor: samples or failure
LinuxCentralProcessor->>ProcLoadavg: parse /proc/loadavg on native failure
ProcLoadavg-->>LinuxCentralProcessor: load averages
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java (1)
507-527: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve
-1for processors whose frequency cannot be read.When one CPU succeeds but another lacks both cpufreq files, the method returns successfully with
0for the unavailable CPU instead of the API's unknown-value sentinel.Proposed fix
protected boolean queryCurrentFreqFromUdev(long[] freqs) { + Arrays.fill(freqs, -1L); long max = 0L; for (String syspath : cpuSyspaths()) { int cpuIdx = ParseUtil.getFirstIntValue(syspath); if (cpuIdx >= 0 && cpuIdx < freqs.length) { - freqs[cpuIdx] = FileUtil.getLongFromFile(syspath + "/cpufreq/scaling_cur_freq"); - if (freqs[cpuIdx] == 0) { - freqs[cpuIdx] = FileUtil.getLongFromFile(syspath + "/cpufreq/cpuinfo_cur_freq"); + long freq = FileUtil.getLongFromFile(syspath + "/cpufreq/scaling_cur_freq"); + if (freq == 0) { + freq = FileUtil.getLongFromFile(syspath + "/cpufreq/cpuinfo_cur_freq"); } - if (max < freqs[cpuIdx]) { - max = freqs[cpuIdx]; + if (freq > 0) { + freqs[cpuIdx] = freq; + max = Math.max(max, freq); } } } if (max > 0L) { for (int i = 0; i < freqs.length; i++) { - freqs[i] *= 1000L; + if (freqs[i] > 0L) { + freqs[i] *= 1000L; + } }🤖 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 `@oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java` around lines 507 - 527, Update queryCurrentFreqFromUdev so processors whose scaling_cur_freq and cpuinfo_cur_freq values are both unavailable retain the API’s -1 sentinel instead of remaining 0. Initialize or assign each unreadable freqs entry to -1 while preserving successful readings, max-frequency detection, and the existing scaling and return behavior.
🤖 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
`@oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java`:
- Around line 239-243: Update cpuSyspathsFromSysfs to enumerate only immediate
children of cpuPath instead of recursively traversing the full sysfs subtree.
Use a non-recursive directory listing and retain only entries whose names match
the direct cpuN pattern, excluding nested topology, cache, and cpufreq paths.
In
`@oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java`:
- Around line 46-54: In LinuxCentralProcessorJNA.java lines 46-54, guard
enumerateNew(), addMatchSubsystem(), scanDevices(), and UdevListEntry.getName()
so any failure returns null and triggers the shared sysfs fallback rather than
throwing or returning an empty result. Apply the same failure-to-null handling
in LinuxCentralProcessorFFM.java lines 55-59 for the corresponding udev
enumeration, subsystem/scan, and entry-name operations.
---
Outside diff comments:
In
`@oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java`:
- Around line 507-527: Update queryCurrentFreqFromUdev so processors whose
scaling_cur_freq and cpuinfo_cur_freq values are both unavailable retain the
API’s -1 sentinel instead of remaining 0. Initialize or assign each unreadable
freqs entry to -1 while preserving successful readings, max-frequency detection,
and the existing scaling and return behavior.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b704ef71-3e2e-4e0b-ba69-8be00ed0b494
📒 Files selected for processing (5)
oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.javaoshi-common/src/main/java/oshi/hardware/common/platform/linux/nativefree/LinuxCentralProcessorNF.javaoshi-common/src/test/java/oshi/hardware/common/platform/linux/LinuxCentralProcessorTest.javaoshi-core-ffm/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorFFM.javaoshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Test JDK 26, macos-latest
- GitHub Check: Test JDK 21, windows-latest
- GitHub Check: Test JDK 26, windows-latest
- GitHub Check: Test JDK 25, macos-latest (+ JNA==FFM)
- GitHub Check: Test JDK 25, windows-latest (+ JNA==FFM)
- GitHub Check: Test JDK 25, ubuntu-latest (+ JNA==FFM)
- GitHub Check: Test JDK 25, FreeBSD (+ JNA==FFM)
- GitHub Check: Analyze (java)
- GitHub Check: Test JDK 21, macos-latest
- GitHub Check: Test JDK 26, ubuntu-latest
- GitHub Check: Test JDK 25, OpenIndiana (+ JNA==FFM)
- GitHub Check: Test JDK 25, OpenBSD (+ JNA==FFM)
- GitHub Check: Lint (Spotless, Checkstyle, ForbiddenAPIs, Javadoc)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-04-12T19:22:09.716Z
Learnt from: dbwiddis
Repo: oshi/oshi PR: 3149
File: oshi-common/src/main/java/oshi/hardware/common/platform/mac/MacCentralProcessor.java:159-163
Timestamp: 2026-04-12T19:22:09.716Z
Learning: When reviewing OSHI hardware platform code that uses or implements `CentralProcessor`/`PhysicalProcessor.getEfficiency()`, remember the API’s counterintuitive semantics: a *higher* efficiency-class value indicates *greater performance* (lower efficiency). P-cores/“performance cores” should therefore have higher values (e.g., 1) than E-cores/“efficiency cores” (e.g., 0), as documented in the `CentralProcessor` Javadoc. Do not flag code as incorrect solely because ARM_P_CORES or Intel P-cores are assigned higher efficiency values.
Applied to files:
oshi-common/src/main/java/oshi/hardware/common/platform/linux/nativefree/LinuxCentralProcessorNF.javaoshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java
📚 Learning: 2026-04-12T19:34:56.879Z
Learnt from: dbwiddis
Repo: oshi/oshi PR: 3149
File: oshi-common/src/main/java/oshi/hardware/common/platform/mac/MacCentralProcessor.java:159-163
Timestamp: 2026-04-12T19:34:56.879Z
Learning: In OSHI’s Java platform implementations, `PhysicalProcessor.getEfficiency()` / `EfficiencyClass` follow the same convention as Windows `PROCESSOR_RELATIONSHIP.EfficiencyClass`: a *higher* numeric efficiency class value represents *greater performance* and *less efficiency* (typically P-cores higher than E-cores). When reviewing, do not treat cases where P-cores are assigned higher efficiency/class values as a bug—this is intentional cross-platform consistency.
Applied to files:
oshi-common/src/main/java/oshi/hardware/common/platform/linux/nativefree/LinuxCentralProcessorNF.javaoshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java
📚 Learning: 2026-07-06T19:25:59.225Z
Learnt from: dbwiddis
Repo: oshi/oshi PR: 3432
File: oshi-common/src/main/java/oshi/util/ExceptionUtil.java:26-56
Timestamp: 2026-07-06T19:25:59.225Z
Learning: In this repo’s SLF4J 1.x-compatible logging code, do not flag cases where a logger call uses SLF4J 1.6+ implicit-cause handling—e.g., `log.error("...{}", value, t)` (or similar with a single `{}` placeholder) where `t` is passed as the final/trailing argument. SLF4J will attach `t` as the log event’s cause with the full stack trace; this is equivalent to SLF4J 2.x’s fluent `atLevel(...).setCause(t).log(...)` and is intentionally used to preserve SLF4J 1.x runtime compatibility.
Applied to files:
oshi-common/src/main/java/oshi/hardware/common/platform/linux/nativefree/LinuxCentralProcessorNF.javaoshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.javaoshi-core-ffm/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorFFM.javaoshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java
📚 Learning: 2026-04-06T04:45:47.644Z
Learnt from: dbwiddis
Repo: oshi/oshi PR: 3121
File: oshi-core/src/main/java/oshi/software/os/linux/LinuxOperatingSystem.java:53-53
Timestamp: 2026-04-06T04:45:47.644Z
Learning: In the oshi/oshi project, only interfaces and classes in the root packages `oshi`, `oshi.hardware`, and `oshi.software.os` are considered the public API. Code in subpackages that represent specific implementations/platforms (e.g., `oshi.software.os.linux`, `oshi.hardware.platform.linux`, and similar subpackages) is implementation detail. When reviewing, do not treat changes to classes in these platform-specific subpackages as public API breaks (for example: making `LinuxOperatingSystem` abstract, narrowing constructor visibility, or adding/removing abstract methods should not be flagged as breaking public API).
Applied to files:
oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java
🔇 Additional comments (5)
oshi-common/src/main/java/oshi/hardware/common/platform/linux/LinuxCentralProcessor.java (1)
207-231: LGTM!Also applies to: 251-272, 539-550, 580-593, 608-615
oshi-common/src/main/java/oshi/hardware/common/platform/linux/nativefree/LinuxCentralProcessorNF.java (1)
15-17: LGTM!Also applies to: 37-45
oshi-core-ffm/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorFFM.java (1)
8-8: LGTM!Also applies to: 30-31, 43-54, 60-71, 74-88
oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java (1)
25-26: LGTM!Also applies to: 36-45, 55-59, 68-69
oshi-common/src/test/java/oshi/hardware/common/platform/linux/LinuxCentralProcessorTest.java (1)
26-27: LGTM!Also applies to: 143-190, 502-529
Comment on lines
+46
to
+54
| UdevEnumerate enumerate = udev.enumerateNew(); | ||
| try { | ||
| enumerate.addMatchSubsystem("cpu"); | ||
| enumerate.scanDevices(); | ||
| for (UdevListEntry entry = enumerate.getListEntry(); entry != null; entry = entry.getNext()) { | ||
| syspaths.add(entry.getName()); | ||
| } | ||
| } finally { | ||
| enumerate.unref(); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve the sysfs fallback when udev enumeration fails.
Guard udev.enumerateNew() / udev_enumerate_new(), the subsystem/scan calls, and entry.getName() in both the JNA and FFM paths; a failure here should return null so the shared sysfs fallback still runs instead of throwing or yielding an empty result.
📍 Affects 2 files
oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java#L46-L54(this comment)oshi-core-ffm/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorFFM.java#L55-L59
🤖 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
`@oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxCentralProcessorJNA.java`
around lines 46 - 54, In LinuxCentralProcessorJNA.java lines 46-54, guard
enumerateNew(), addMatchSubsystem(), scanDevices(), and UdevListEntry.getName()
so any failure returns null and triggers the shared sysfs fallback rather than
throwing or returning an empty result. Apply the same failure-to-null handling
in LinuxCentralProcessorFFM.java lines 55-59 for the corresponding udev
enumeration, subsystem/scan, and entry-name operations.
Collapse the JNA/FFM/native-free divergence in the Linux central processor to two hooks, moving all shared logic into LinuxCentralProcessor: - enumerateCpuSyspathsViaUdev() returns the CPU syspaths via udev, or null when udev is unavailable so the base falls back to a sysfs directory scan. This is the only genuinely udev-specific operation (JNA UdevContext vs FFM UdevFunctions); everything downstream is now shared. - getloadavgNative() performs the native getloadavg; the native-free backend returns -1 so the base falls back to /proc/loadavg. The base now implements readTopologyWithUdev (via buildTopology over the shared uevent-based reader), queryCurrentFreqFromUdev, queryMaxFreqFromUdev, and getSystemLoadAverage once. getSystemLoadAverage is all-or-nothing: a partial native result blanks the whole array (a partial result is more likely a misread than a real load average), matching the other platforms, with a /proc/loadavg fallback when the native call is unavailable. Two internal behavior notes, both preserving observable output: - Topology MODALIAS is now read from the cpu uevent file (the source the sysfs path already used and that udev itself derives the property from) rather than the udev device property. - The no-udev frequency path now enumerates CPUs with the same sysfs scan used for topology. Twins shrink substantially (JNA 187->70, FFM 200->95, NF 51->48). Add stub-based getSystemLoadAverage coverage (full, all-or-nothing partial, bounds), gated to Linux because constructing a LinuxCentralProcessor initializes the Linux-only SysPath, plus cpuSyspathsFromSysfs enumeration tests.
dbwiddis
force-pushed
the
linux-cpu-dedup
branch
from
July 15, 2026 06:29
651942b to
2eeb289
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3463 +/- ##
============================================
+ Coverage 74.88% 74.92% +0.03%
Complexity 76 76
============================================
Files 587 587
Lines 21715 21603 -112
Branches 3638 3611 -27
============================================
- Hits 16262 16185 -77
+ Misses 3446 3434 -12
+ Partials 2007 1984 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Completes the CentralProcessor cross-OS consistency/dedup arc (after Windows #3459, FreeBSD/DragonFly #3460, OpenBSD #3461, and macOS #3462), now for Linux.
What
The Linux central processor's JNA/FFM/native-free backends duplicated ~130 lines of near-identical udev topology and frequency logic, plus separate
getSystemLoadAverageoverrides. The only part that genuinely differs between JNA and FFM is the udev enumeration itself; everything downstream (syspath parsing, cpufreq file reads, topology building, MODALIAS) is plain Java. So the entire divergence collapses to two hooks:enumerateCpuSyspathsViaUdev()— returns the CPU syspaths via udev, ornullwhen udev is unavailable so the base falls back to a sysfs directory scan. JNA (UdevContext), FFM (UdevFunctions), native-free (null).getloadavgNative()— nativegetloadavg; the native-free backend returns-1so the base falls back to/proc/loadavg.The base
LinuxCentralProcessornow implements topology, current-frequency, max-frequency, and load-average once.getSystemLoadAverageis all-or-nothing (a partial native result blanks the whole array — a partial read is more likely a misread than a real load average), consistent with the other platforms, with a/proc/loadavgfallback.Twins shrink: JNA 187→70, FFM 200→95, NF 51→48 (net −114 lines).
Behavior notes (both preserve observable output)
ueventfile — the source the sysfs path already used and that udev itself derives the property from — rather than the udev device property.Tests
Added stub-based
getSystemLoadAveragecoverage (full result, all-or-nothing partial, bounds), gated@EnabledOnOs(LINUX)because constructing aLinuxCentralProcessorinitializes the Linux-onlySysPath; pluscpuSyspathsFromSysfsenumeration tests that run everywhere. The existing topology tests pass unchanged through the new sharedbuildTopology.Validated with a clean full build on macOS; the Linux runtime paths (udev/freq/topology) are exercised by the Linux CI matrix (JNA + FFM + native-free). No CHANGELOG — no user-facing behavior change.
Summary by CodeRabbit
/proc, including stricter handling of partial/invalid results./procfor these metrics.