Jank fix: thread priorities — raise vsync waker, demote decoders
Slint's android-activity backend wakes the main loop from a dedicated "slint vsync" Choreographer thread while any animation is active (i-slint-backend-android-activity/vsync.rs). Android schedules that thread at nice +10 (lowest on device), while our 4 turbojpeg workers inherited the UI thread's nice -10. Under CPU bursts the +10 waker starves and the main loop's next wakeup lands 3-4 frames late: 25-33 ms of pure wake latency at 120 Hz, with zero cost in our tick (d=7-34 us) — exactly the phone stall signature (measured: 28-39 ms gaps, catch-up events). - prio module: setpriority FFI (self + by-tid), raise_slint_vsync() - thumb workers + dispatcher: nice +2 at spawn (ahead of background apps, behind the UI's -10) - android_main: prio-fix thread raises "slint vsync" to nice 0 with a readback log - verified on emulator: thumb-* hold nice 2, slint vsync holds 0 while foreground (Android re-nices backgrounded processes — expected) - emulator scroll cadence unchanged (llvmpipe rasterization ceiling, not a priority issue) — phone is the acceptance target, A/B test planned for when it is back online Also: drop unused android_logger dependency (nblog replaced it).
This commit is contained in:
parent
a6b5e31924
commit
9c0525e93f
28
Cargo.lock
generated
28
Cargo.lock
generated
|
|
@ -215,23 +215,6 @@ version = "0.2.2"
|
|||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "fc7eb209b1518d6bb87b283c20095f5228ecda460da70b44f0802523dea6da04"
|
||||
|
||||
[[package]]
|
||||
name = "android_log-sys"
|
||||
version = "0.3.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "84521a3cf562bc62942e294181d9eef17eb38ceb8c68677bc49f144e4c3d4f8d"
|
||||
|
||||
[[package]]
|
||||
name = "android_logger"
|
||||
version = "0.14.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "05b07e8e73d720a1f2e4b6014766e6039fd2e96a4fa44e2a78d0e1fa2ff49826"
|
||||
dependencies = [
|
||||
"android_log-sys",
|
||||
"env_filter",
|
||||
"log",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "android_system_properties"
|
||||
version = "0.1.6"
|
||||
|
|
@ -1184,16 +1167,6 @@ dependencies = [
|
|||
"syn 2.0.119",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "env_filter"
|
||||
version = "0.1.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "1bf3c259d255ca70051b30e2e95b5446cdb8949ac4cd22c0d7fd634d89f568e2"
|
||||
dependencies = [
|
||||
"log",
|
||||
"regex",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "equivalent"
|
||||
version = "1.0.3"
|
||||
|
|
@ -1553,7 +1526,6 @@ dependencies = [
|
|||
name = "gallery"
|
||||
version = "0.1.0"
|
||||
dependencies = [
|
||||
"android_logger",
|
||||
"image",
|
||||
"kamadak-exif",
|
||||
"log",
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ crate-type = ["cdylib", "lib"]
|
|||
[dependencies]
|
||||
slint = { version = "1.18.1", features = ["backend-android-activity-06"] }
|
||||
log = "0.4"
|
||||
android_logger = { version = "0.14", target-family = "android" }
|
||||
rusqlite = { version = "0.41.0", features = ["bundled"] }
|
||||
sha1 = "0.11.0"
|
||||
turbojpeg = { version = "1.5.1", features = ["cmake"] }
|
||||
|
|
|
|||
19
HANDOFF.md
19
HANDOFF.md
|
|
@ -396,10 +396,23 @@ phone (Pixel 9 Pro, 120 Hz). Emulator repro via programmatic scroll hook.
|
|||
3. Compare against Fossify (installed on the phone, user says smooth):
|
||||
`dumpsys SurfaceFlinger --latency` on both apps' layers during identical
|
||||
scrolls.
|
||||
4. Candidate code changes (after the A/B): GRID_TARGET 500→400 (one-time
|
||||
4. **Priority fix already applied** (commit after this edit):
|
||||
- `thumb-0..3` + `thumb-dispatch` demoted to nice +2 (they inherited the
|
||||
UI's -10 and could preempt it during turbojpeg bursts).
|
||||
- Slint's `slint vsync` Choreographer waker (created at nice +10 by
|
||||
Android — the ONLY wakeup source for the main loop while animating,
|
||||
see i-slint-backend-android-activity/vsync.rs) is raised to nice 0 by
|
||||
a `prio-fix` thread in android_main (log: "slint vsync raised to nice
|
||||
0"). A starved waker delays the main loop's wakeup by 3–4 frames at
|
||||
120 Hz = 25–33 ms with zero tick cost — the measured phone signature.
|
||||
Verified on emulator: nice holds at 0 while foreground; Android
|
||||
re-nices backgrounded processes (expected, irrelevant).
|
||||
- Verification on phone: check the two log lines, then
|
||||
`for t in /proc/$(pidof wow.gmp.gallery)/task/*; do ...` and confirm
|
||||
thumb-* at 2, slint vsync at 0, before/while the user scrolls.
|
||||
5. Candidate code changes (after the A/B): GRID_TARGET 500→400 (one-time
|
||||
cache invalidation — don't do it before knowing the culprit), 1 painted
|
||||
cell/tick instead of 2, worker nice level (+2) so turbojpeg bursts
|
||||
can't preempt the UI thread.
|
||||
cell/tick instead of 2.
|
||||
|
||||
## 9. Milestones
|
||||
|
||||
|
|
|
|||
98
src/lib.rs
98
src/lib.rs
|
|
@ -80,6 +80,81 @@ pub mod images;
|
|||
pub mod scan;
|
||||
pub mod thumb;
|
||||
|
||||
/// Thread-priority helpers (jank fix): the app process runs at nice -10,
|
||||
/// which every spawned thread inherits — turbojpeg decode bursts then run at
|
||||
/// the SAME priority as the UI thread and can starve Slint's "slint vsync"
|
||||
/// Choreographer thread (which Android drops to nice +10). A starved vsync
|
||||
/// waker delays the main loop's wakeup by 3–4 frames at 120 Hz = 25–33 ms of
|
||||
/// pure wake latency (measured phone signature: stall with zero tick cost).
|
||||
/// Decoders are dropped to nice +2; the vsync waker is raised to nice 0.
|
||||
/// Raising another thread to a nice value ≥ the caller's (-10) is allowed
|
||||
/// unprivileged; lowering one's own is always allowed.
|
||||
#[cfg(target_os = "android")]
|
||||
pub mod prio {
|
||||
extern "C" {
|
||||
fn setpriority(which: i32, who: u32, prio: i32) -> i32;
|
||||
}
|
||||
const PRIO_PROCESS: i32 = 0;
|
||||
pub fn set_self_nice(nice: i32) {
|
||||
unsafe {
|
||||
if setpriority(PRIO_PROCESS, 0, nice) != 0 {
|
||||
log::warn!("setpriority(self, {nice}) failed (errno {})", std::io::Error::last_os_error().raw_os_error().unwrap_or(-1));
|
||||
}
|
||||
}
|
||||
}
|
||||
pub fn set_tid_nice(tid: u32, nice: i32) -> bool {
|
||||
unsafe { setpriority(PRIO_PROCESS, tid, nice) == 0 }
|
||||
}
|
||||
/// Raise the Slint vsync waker thread ("slint vsync") from nice +10 to
|
||||
/// `nice`. Returns the tid on success.
|
||||
pub fn raise_slint_vsync(nice: i32) -> Option<u32> {
|
||||
let Ok(rd) = std::fs::read_dir("/proc/self/task") else {
|
||||
return None;
|
||||
};
|
||||
for e in rd.flatten() {
|
||||
let tid_name = e.file_name();
|
||||
let Ok(c) = std::fs::read_to_string(e.path().join("comm")) else {
|
||||
continue;
|
||||
};
|
||||
if c.trim() != "slint vsync" {
|
||||
continue;
|
||||
}
|
||||
let Some(tid_str) = tid_name.to_str() else {
|
||||
continue;
|
||||
};
|
||||
let Ok(tid) = tid_str.parse::<u32>() else {
|
||||
continue;
|
||||
};
|
||||
if set_tid_nice(tid, nice) {
|
||||
// Read back the kernel's value — Android may be re-nicing us.
|
||||
let stat = std::fs::read_to_string(format!("/proc/self/task/{tid}/stat")).ok();
|
||||
if let Some(s) = stat {
|
||||
// comm ("slint vsync") contains a space — parse after the
|
||||
// closing ')'; nice is the 17th field after it.
|
||||
let rest = s.rsplit(')').next().unwrap_or("");
|
||||
let f: Vec<&str> = rest.split_whitespace().collect();
|
||||
if let Some(n) = f.get(16) {
|
||||
log::info!("slint vsync tid {tid}: set nice {nice} -> readback {n}");
|
||||
}
|
||||
}
|
||||
return Some(tid);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
/// Desktop no-ops so thumb.rs can call these unconditionally.
|
||||
#[cfg(not(target_os = "android"))]
|
||||
pub mod prio {
|
||||
pub fn set_self_nice(_nice: i32) {}
|
||||
pub fn set_tid_nice(_tid: u32, _nice: i32) -> bool {
|
||||
false
|
||||
}
|
||||
pub fn raise_slint_vsync(_nice: i32) -> Option<u32> {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::ops::Range;
|
||||
use std::path::Path;
|
||||
|
|
@ -564,7 +639,7 @@ fn on_scroll(app: &AppState, y: f32, height: f32, width: f32) {
|
|||
/// !Send (UI thread only): we leak the one handle for the process lifetime
|
||||
/// (8 bytes).
|
||||
/// Tick counter for rate-limiting the debug coverage log (debug level: the
|
||||
/// lines are only visible when the android_logger filter is Debug).
|
||||
/// lines are only visible at log level Debug).
|
||||
static DBG_TICKS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
|
||||
|
||||
fn start_drain_timer(weak: &slint::Weak<Main>) {
|
||||
|
|
@ -1001,6 +1076,27 @@ fn android_main(app: slint::android::AndroidApp) {
|
|||
})
|
||||
.unwrap();
|
||||
|
||||
// Jank fix: the "slint vsync" Choreographer waker thread is created when
|
||||
// run() starts the event loop and Android schedules it at nice +10 — the
|
||||
// lowest priority on device, so CPU bursts (decoders, renders) starve it
|
||||
// and the main loop's next wakeup is delayed by 3–4 frames (25–33 ms at
|
||||
// 120 Hz; the measured phone stall signature). Raise it to nice 0 from a
|
||||
// background thread (it only exists once run() is looping).
|
||||
std::thread::Builder::new()
|
||||
.name("prio-fix".into())
|
||||
.spawn(|| {
|
||||
std::thread::sleep(std::time::Duration::from_millis(400));
|
||||
for _ in 0..50 {
|
||||
if let Some(tid) = prio::raise_slint_vsync(0) {
|
||||
log::info!("slint vsync raised to nice 0 (tid {tid})");
|
||||
return;
|
||||
}
|
||||
std::thread::sleep(std::time::Duration::from_millis(200));
|
||||
}
|
||||
log::warn!("slint vsync thread not found; priority left at +10");
|
||||
})
|
||||
.ok();
|
||||
|
||||
let main_window = Main::new().unwrap();
|
||||
start_scan(&main_window);
|
||||
main_window.run().unwrap();
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use gallery::Main;
|
||||
use slint::ComponentHandle;
|
||||
|
||||
/// Minimal eprintln logger for desktop dev (Android uses android_logger).
|
||||
/// Minimal eprintln logger for desktop dev (Android uses the non-blocking nblog).
|
||||
struct Stdout;
|
||||
|
||||
impl log::Log for Stdout {
|
||||
|
|
|
|||
|
|
@ -167,6 +167,10 @@ impl Engine {
|
|||
std::thread::Builder::new()
|
||||
.name(format!("thumb-{i}"))
|
||||
.spawn(move || {
|
||||
// Jank fix: decode bursts must not preempt the UI thread
|
||||
// (same process priority by default) or starve the vsync
|
||||
// waker. +2 keeps them ahead of background apps.
|
||||
crate::prio::set_self_nice(2);
|
||||
worker_loop(
|
||||
&wq_t, res_tx, gen, in_flight, done, hits, errs, cache_dir, target, closed_w,
|
||||
)
|
||||
|
|
@ -184,6 +188,7 @@ impl Engine {
|
|||
std::thread::Builder::new()
|
||||
.name("thumb-dispatch".into())
|
||||
.spawn(move || {
|
||||
crate::prio::set_self_nice(2);
|
||||
dispatch_loop(d_jobs, d_cv, d_closed, wqs, d_res, d_inflight, max_worker_queue)
|
||||
})
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user