diff --git a/Cargo.lock b/Cargo.lock index e10f55e..fcffd35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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", diff --git a/Cargo.toml b/Cargo.toml index 5c0831f..03f05f9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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"] } diff --git a/HANDOFF.md b/HANDOFF.md index 0f8efca..270342c 100644 --- a/HANDOFF.md +++ b/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 diff --git a/src/lib.rs b/src/lib.rs index 2c1ef1a..28aded1 100644 --- a/src/lib.rs +++ b/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 { + 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::() 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 { + 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
) { @@ -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(); diff --git a/src/main.rs b/src/main.rs index b07ce5c..d4bcec3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -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 { diff --git a/src/thumb.rs b/src/thumb.rs index 2dd2905..96ef9e6 100644 --- a/src/thumb.rs +++ b/src/thumb.rs @@ -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())?;