Repository navigation
Power tuner counts repeated frames; game pinned to the clusters above the little one - #2321
Closed
utkarshdalal wants to merge 1 commit into
Closed
utkarshdalal wants to merge 1 commit into
utkarshdalal wants to merge 1 commit into
Conversation
… the little one A capped game read 59.9 fps with a p95 of one period as steady while it repeated a frame every few seconds on trimmed clocks (RP6, SteamWorld Dig 2: perf cluster at 499 MHz, 25 repeats in 70 s). The tuner now reads frame intervals over 1.5 target periods in the last 10 s from the frame ring: any repeat blocks the next trim and fails a trim watch, two or more step the last trim back once per window. The game was pinned to all cores but the two slowest, which left it a little core (11 ms per frame there against 4.4 ms on the prime core). It now runs on every cluster above the lowest one when those hold at least 4 cores.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt:294">
P2: A repeat-triggered watch undo never starts the repeat cooldown. The same count can undo another trim on the next cycle; set the cooldown whenever a repeat causes a watch undo.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt:302">
P1: Repeat handling runs too late: the out-of-target path can harvest another trim while repeats are present, and `harvestWatch()` does not treat a repeat as degradation. Handle repeats before the raise/harvest path and in the harvest watch, while preserving the watched domain for an undo.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt:330">
P2: This history remains stale after an ordinary raise. Reconcile the domain’s trim history whenever a raise reverses it, or track active trim steps so a repeat undo cannot raise a cap unrelated to the latest live trim.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt:165">
P2: The new 10-second repeat history spans background pauses, so the first frame after a short pause is counted as a repeated frame. Clear the repeat baseline on pause/resume or exclude the interval crossing the pause before feeding the tuner.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt:173">
P2: This applies the current frame-generation stride to timestamps recorded before a stride change, so changing LSFG from 1× to 2× can report false repeats for the remainder of the 10-second window and make the tuner hold or undo a valid trim. Reset or segment the repeat window whenever the stride changes, or retain the stride with each timestamp.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt:220">
P2: The new repeat metric is published and logged to logcat but omitted from the JSONL records used for offline analysis. Add `repeatedFrameCount` to `toJsonLine` so session files preserve the tuner input.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:1093">
P2: The new pinning path assumes cluster labels are valid even when frequency discovery failed. Fall back to the previous all-but-two policy unless the driver confirms frequency-based classification, or expose and validate that status before selecting clusters.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:1106">
P3: `clusterGameCores` does not leave a 6+2 layout on two cores: the fallback returns six cores. Reword this rationale to describe the rejected two-core candidate.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:1112">
P2: This branch can make game and background affinities overlap on devices whose lowest-frequency cluster has one core. Require at least two cores in the lowest cluster before using the cluster-based branch, otherwise retain the all-but-two fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| return decision(TunerAction.HOLD, watched, 0, "watch:${watched.key}") | ||
| } | ||
|
|
||
| val repeated = input.repeatedFrameCount |
Contributor
There was a problem hiding this comment.
P1: Repeat handling runs too late: the out-of-target path can harvest another trim while repeats are present, and harvestWatch() does not treat a repeat as degradation. Handle repeats before the raise/harvest path and in the harvest watch, while preserving the watched domain for an undo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt, line 302:
<comment>Repeat handling runs too late: the out-of-target path can harvest another trim while repeats are present, and `harvestWatch()` does not treat a repeat as degradation. Handle repeats before the raise/harvest path and in the harvest watch, while preserving the watched domain for an undo.</comment>
<file context>
@@ -258,12 +291,29 @@ class TunerDecisionEngine(
return decision(TunerAction.HOLD, watched, 0, "watch:${watched.key}")
}
+ val repeated = input.repeatedFrameCount
+ if (repeated > 0) {
+ trimQualifiedCycles = 0
</file context>
| trimQualifiedCycles = 0 | ||
| watchedDomain = domain | ||
| watchCyclesRemaining = tuning.trimWatchCycles | ||
| trimHistory.add(domain) |
Contributor
There was a problem hiding this comment.
P2: This history remains stale after an ordinary raise. Reconcile the domain’s trim history whenever a raise reverses it, or track active trim steps so a repeat undo cannot raise a cap unrelated to the latest live trim.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt, line 330:
<comment>This history remains stale after an ordinary raise. Reconcile the domain’s trim history whenever a raise reverses it, or track active trim steps so a repeat undo cannot raise a cap unrelated to the latest live trim.</comment>
<file context>
@@ -277,6 +327,7 @@ class TunerDecisionEngine(
trimQualifiedCycles = 0
watchedDomain = domain
watchCyclesRemaining = tuning.trimWatchCycles
+ trimHistory.add(domain)
return decision(TunerAction.TRIM, domain, 1, "trim:${domain.key}")
}
</file context>
| cpuTempC = cpuTemp?.celsius, | ||
| gpuTempC = gpuTemp?.celsius, | ||
| thermalHeadroom = ThermalHeadroom.latest, | ||
| repeatedFrameCount = repeatedFrames, |
Contributor
There was a problem hiding this comment.
P2: The new repeat metric is published and logged to logcat but omitted from the JSONL records used for offline analysis. Add repeatedFrameCount to toJsonLine so session files preserve the tuner input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt, line 220:
<comment>The new repeat metric is published and logged to logcat but omitted from the JSONL records used for offline analysis. Add `repeatedFrameCount` to `toJsonLine` so session files preserve the tuner input.</comment>
<file context>
@@ -205,6 +217,7 @@ object PerformanceMetricsCollector {
cpuTempC = cpuTemp?.celsius,
gpuTempC = gpuTemp?.celsius,
thermalHeadroom = ThermalHeadroom.latest,
+ repeatedFrameCount = repeatedFrames,
)
</file context>
| repeatScratch, | ||
| repeatCount, | ||
| slowFrameThresholdNs(), | ||
| PowerManager.frameSampleStride, |
Contributor
There was a problem hiding this comment.
P2: This applies the current frame-generation stride to timestamps recorded before a stride change, so changing LSFG from 1× to 2× can report false repeats for the remainder of the 10-second window and make the tuner hold or undo a valid trim. Reset or segment the repeat window whenever the stride changes, or retain the stride with each timestamp.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt, line 173:
<comment>This applies the current frame-generation stride to timestamps recorded before a stride change, so changing LSFG from 1× to 2× can report false repeats for the remainder of the 10-second window and make the tuner hold or undo a valid trim. Reset or segment the repeat window whenever the stride changes, or retain the stride with each timestamp.</comment>
<file context>
@@ -160,6 +162,16 @@ object PerformanceMetricsCollector {
+ repeatScratch,
+ repeatCount,
+ slowFrameThresholdNs(),
+ PowerManager.frameSampleStride,
+ )
</file context>
| */ | ||
| private fun gameCores(pserver: PServerDriver): List<Int> { | ||
| val all = allCpuCoresSorted(pserver) | ||
| return clusterGameCores( |
Contributor
There was a problem hiding this comment.
P2: The new pinning path assumes cluster labels are valid even when frequency discovery failed. Fall back to the previous all-but-two policy unless the driver confirms frequency-based classification, or expose and validate that status before selecting clusters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 1093:
<comment>The new pinning path assumes cluster labels are valid even when frequency discovery failed. Fall back to the previous all-but-two policy unless the driver confirms frequency-based classification, or expose and validate that status before selecting clusters.</comment>
<file context>
@@ -1084,11 +1085,31 @@ object PowerManager {
*/
private fun gameCores(pserver: PServerDriver): List<Int> {
- val all = allCpuCoresSorted(pserver)
+ return clusterGameCores(
+ listOf(
+ pserver.getCpuCoresByCluster(PServerDriver.CpuCluster.EFFICIENCY),
</file context>
| val clusters = clustersLowToHigh.filter { it.isNotEmpty() } | ||
| val all = clusters.flatten() | ||
| val aboveLowest = clusters.drop(1).flatten() | ||
| if (clusters.size >= 2 && aboveLowest.size >= MIN_GAME_CLUSTER_CORES) return aboveLowest |
Contributor
There was a problem hiding this comment.
P2: This branch can make game and background affinities overlap on devices whose lowest-frequency cluster has one core. Require at least two cores in the lowest cluster before using the cluster-based branch, otherwise retain the all-but-two fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 1112:
<comment>This branch can make game and background affinities overlap on devices whose lowest-frequency cluster has one core. Require at least two cores in the lowest cluster before using the cluster-based branch, otherwise retain the all-but-two fallback.</comment>
<file context>
@@ -1084,11 +1085,31 @@ object PowerManager {
+ val clusters = clustersLowToHigh.filter { it.isNotEmpty() }
+ val all = clusters.flatten()
+ val aboveLowest = clusters.drop(1).flatten()
+ if (clusters.size >= 2 && aboveLowest.size >= MIN_GAME_CLUSTER_CORES) return aboveLowest
return if (all.size <= 2) all else all.drop(2)
}
</file context>
Suggested change
| if (clusters.size >= 2 && aboveLowest.size >= MIN_GAME_CLUSTER_CORES) return aboveLowest | |
| if (clusters.size >= 2 && clusters.first().size >= 2 && aboveLowest.size >= MIN_GAME_CLUSTER_CORES) return aboveLowest |
| clearWatch() | ||
| trimQualifiedCycles = 0 | ||
| return decision(TunerAction.UNDO, watched, 1, "undo:${watched.key}") | ||
| forgetTrim(watched) |
Contributor
There was a problem hiding this comment.
P2: A repeat-triggered watch undo never starts the repeat cooldown. The same count can undo another trim on the next cycle; set the cooldown whenever a repeat causes a watch undo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/autotuning/TunerDecisionEngine.kt, line 294:
<comment>A repeat-triggered watch undo never starts the repeat cooldown. The same count can undo another trim on the next cycle; set the cooldown whenever a repeat causes a watch undo.</comment>
<file context>
@@ -258,12 +291,29 @@ class TunerDecisionEngine(
clearWatch()
trimQualifiedCycles = 0
- return decision(TunerAction.UNDO, watched, 1, "undo:${watched.key}")
+ forgetTrim(watched)
+ val cause = if (input.repeatedFrameCount > 0) " ${repeatLabel(input)}" else ""
+ return decision(TunerAction.UNDO, watched, 1, "undo:${watched.key}$cause")
</file context>
Suggested change
| forgetTrim(watched) | |
| forgetTrim(watched) | |
| if (input.repeatedFrameCount > 0) { | |
| repeatUndoCooldownCycles = TunerThresholds.REPEAT_UNDO_COOLDOWN_CYCLES | |
| } |
| deltaScratch, | ||
| PowerManager.frameSampleStride, | ||
| ) | ||
| val repeatCount = FrameTimeRing.copySince( |
Contributor
There was a problem hiding this comment.
P2: The new 10-second repeat history spans background pauses, so the first frame after a short pause is counted as a repeated frame. Clear the repeat baseline on pause/resume or exclude the interval crossing the pause before feeding the tuner.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/metrics/PerformanceMetricsCollector.kt, line 165:
<comment>The new 10-second repeat history spans background pauses, so the first frame after a short pause is counted as a repeated frame. Clear the repeat baseline on pause/resume or exclude the interval crossing the pause before feeding the tuner.</comment>
<file context>
@@ -160,6 +162,16 @@ object PerformanceMetricsCollector {
deltaScratch,
PowerManager.frameSampleStride,
)
+ val repeatCount = FrameTimeRing.copySince(
+ now - TunerThresholds.REPEAT_WINDOW_MS * 1_000_000L,
+ repeatScratch,
</file context>
| * Game cores from clusters ordered lowest to highest frequency: the cores of every | ||
| * cluster above the lowest one. Falls back to all cores but the 2 slowest when there is | ||
| * only one cluster, or when the clusters above the lowest hold fewer than | ||
| * [MIN_GAME_CLUSTER_CORES] cores (a 6+2 part would leave a Wine game on 2 cores). |
Contributor
There was a problem hiding this comment.
P3: clusterGameCores does not leave a 6+2 layout on two cores: the fallback returns six cores. Reword this rationale to describe the rejected two-core candidate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 1106:
<comment>`clusterGameCores` does not leave a 6+2 layout on two cores: the fallback returns six cores. Reword this rationale to describe the rejected two-core candidate.</comment>
<file context>
@@ -1084,11 +1085,31 @@ object PowerManager {
+ * Game cores from clusters ordered lowest to highest frequency: the cores of every
+ * cluster above the lowest one. Falls back to all cores but the 2 slowest when there is
+ * only one cluster, or when the clusters above the lowest hold fewer than
+ * [MIN_GAME_CLUSTER_CORES] cores (a 6+2 part would leave a Wine game on 2 cores).
+ */
+ internal fun clusterGameCores(clustersLowToHigh: List<List<Int>>): List<Int> {
</file context>
Suggested change
| * [MIN_GAME_CLUSTER_CORES] cores (a 6+2 part would leave a Wine game on 2 cores). | |
| * [MIN_GAME_CLUSTER_CORES] cores (selecting only the 2-core high cluster would be too restrictive on a 6+2 part). |
Owner
Author
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
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.
Stacked on #2320 (base
gamescope-real-vsync-payload-20261102-2): merge #2320 first, then retarget this to master.Description
What the tuner got wrong. On a Retroid Pocket 6 (Snapdragon 8 Gen 2: c0-2 little, c3-6 big, c7 prime) with SteamWorld Dig 2 under gamescope (cap 60, 60 Hz panel), the ClusterTuner treated 59-60 fps with a p95 of 17 ms as steady and kept trimming. It ended with the perf cluster c3-6 pinned at 499 MHz (min = max) and c7 capped at 1.25-1.7 GHz. Meanwhile PowerManager pinned the game to "all cores but the two slowest" (2-7), so the main thread usually ran on c2 (a little core) or a 0.5 GHz big core. There a frame costs about 11 ms, against 4.4 ms on the prime core, and the game repeats a frame every few seconds. Neither fps nor p95 moves enough to show it.
Change 1: repeated frames count against "steady".
onFramePublishedunder gamescope), how many intervals in the last 10 s were longer than 1.5 × the target period. The count is passed to the tuner asrepeatedFrameCount.Change 2: game core set by cluster.
Neither change adds a setting.
Measurement. RP6, SWD2, gamescope cap 60 @ 60 Hz, power control on, payload 20261102.2, same scene (mine entrance), 180 s settle + 90 s capture.
Caveat: the run with this PR started from the tuner state file the control run left behind (prime 1.0 / perf 0.61 GHz, against 2.73 / 2.32 GHz for the control), and the prime cap was still being trimmed during the capture.
Reference, power control off: 59.968 fps, 1 repeat in 90 s.
Open knobs.
Log lines to look for.
PowerManager: Game core set 3, 4, 5, 6, 7 (clusters efficiency=[0, 1, 2] performance=[3, 4, 5, 6] prime=[7], background=0, 1)PowerTuner: hold repeat-hold repeated=N in 10s: …PowerTuner: undo repeat-undo:<prime|perf|gpu> repeated=N in 10s: …PowerTuner: undo undo:<domain> repeated=N in 10s(repeat during a trim watch)repeated=N in 10son every applied tuner line and on the periodicPowerMetricsline"repeated":Nin the tuner jsonlTests. 13 new unit tests, all passing:
TunerRepeatedFramesTest(6): trim on a clean render, hold on one repeat, undo on a repeat during the watch, one step-back per window, a single repeat holds without stepping back, warm-start fallbackGameCoreSetTest(4): 3+4+1 → 3-7, 4+4 → 4-7, 6+2 falls back, single cluster falls backRepeatedFrameCountTest(3)Recording
None; frame-pacing change, numbers above.
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
The power tuner now detects repeated frames that FPS and p95 miss, so it can respond to stutter instead of continuing to trim clocks. Game threads now run on clusters above the lowest-frequency cluster when those clusters provide enough cores.
Written for commit 65e3137. Summary will update on new commits.