Give memory back in the order the user will miss it least
FR-PLAT-AND-5. Android asks for memory back through onTrimMemory and kills the process if it is not given; until now nothing listened, so the answer was always "no". A tiered registry answers instead: GPU caches first, then proxies, then thumbnails, driven from android_main on MainEvent::LowMemory and MainEvent::Stop. The order is the argument. A backgrounded app has no window to draw and therefore no use for a render pipeline, while its thumbnails are exactly what the user will be looking at half a second after they come back -- so going into the background frees only the GPU tier, and only being measured against death frees everything. Sinks register beside the cache they free and hold weak handles, so the registry cannot keep a controller -- and every decoded portrait in it -- alive past the interface it belonged to. `try_borrow_mut` and skip: a warning can land mid-render, freeing textures under the code drawing with them is worse than missing one, and a warning not acted on is always followed by another. The GPU test is the one that matters: an eviction must change no pixel. A freed intermediate pool whose `colour_key` promise still stands renders an empty texture, and nothing else would have caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -2943,6 +2943,33 @@ impl DevelopSession {
|
||||
Ok((image, rw, rh))
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5 | NFR-RES-1
|
||||
/// Give back the GPU memory this session is holding only to be fast.
|
||||
///
|
||||
/// The edit is untouched: the graph and its history are CPU-side by
|
||||
/// design (ARCH §6.1), so the photograph, the undo stack and the viewport
|
||||
/// all survive and the next frame simply costs what the first one did.
|
||||
///
|
||||
/// # What is not released, and what it is waiting on
|
||||
///
|
||||
/// The demosaiced source is the largest single allocation a session holds
|
||||
/// — a 24 MP frame is about 190 MB of `Rgba16Float` — and it is
|
||||
/// deliberately kept. Dropping it would need the session to be able to
|
||||
/// rebuild itself from the file, and rebuilding a session from a durable
|
||||
/// record is FR-PLAT-AND-3, which is not built. Freeing it now would not
|
||||
/// be an eviction; it would be closing the photograph without telling
|
||||
/// anyone. Likewise the subject distance fields and the segmentation map:
|
||||
/// each is guarded by a key recording what it was built from, and freeing
|
||||
/// one without invalidating its key is the failure `AdjustPass` documents
|
||||
/// under `colour_key`.
|
||||
///
|
||||
/// So this is the part of the GPU tier that can be given back and asked
|
||||
/// for again with no other machinery, which is exactly as far as an
|
||||
/// eviction should go.
|
||||
pub fn release_gpu_caches(&mut self) {
|
||||
self.adjust.release_caches();
|
||||
}
|
||||
|
||||
/// The displayed size, for sizing the viewport.
|
||||
///
|
||||
/// The *framed* size, not the sensor's: cropping and quarter turns change
|
||||
|
||||
@@ -105,6 +105,21 @@ impl IdentityController {
|
||||
fn clear_picks(&self) {
|
||||
self.picked.borrow_mut().clear();
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5 | NFR-RES-1
|
||||
/// Drop the decoded rail portraits.
|
||||
///
|
||||
/// The one in-memory image cache in this crate that is unbounded by
|
||||
/// anything but the library: one decoded portrait per person, kept for as
|
||||
/// long as the person exists. On a library with a few hundred named people
|
||||
/// that is worth tens of megabytes of nothing but a saved decode.
|
||||
///
|
||||
/// Costless to lose. `refresh` rebuilds any portrait it does not find, so
|
||||
/// the only consequence is the JPEG decode this cache exists to skip, and
|
||||
/// only for the people the rail is actually showing at the time.
|
||||
pub fn clear_covers(&self) {
|
||||
self.covers.borrow_mut().clear();
|
||||
}
|
||||
}
|
||||
|
||||
/// Push the people rail and the face grid into the window.
|
||||
|
||||
@@ -38,6 +38,7 @@ mod library_ui;
|
||||
#[cfg(live_style)]
|
||||
mod live_style;
|
||||
mod masks_ui;
|
||||
pub mod memory;
|
||||
mod net_runtime;
|
||||
mod presets;
|
||||
mod remote;
|
||||
@@ -1003,6 +1004,22 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// faces are ticked for a split.
|
||||
let identity = std::rc::Rc::new(identity_ui::IdentityController::new());
|
||||
|
||||
// TRACES: FR-PLAT-AND-5
|
||||
// The thumbnail tier. Registered here, beside the thing it frees, so that
|
||||
// a controller which grows another cache is one line from offering it up.
|
||||
//
|
||||
// Weak, not strong: `run` returns when the window closes, and a registry
|
||||
// holding the last reference to a controller would keep it — and every
|
||||
// decoded portrait in it — alive past the interface it belonged to.
|
||||
{
|
||||
let identity = std::rc::Rc::downgrade(&identity);
|
||||
memory::evict_at(memory::Tier::Thumbnails, move || {
|
||||
if let Some(ctl) = identity.upgrade() {
|
||||
ctl.clear_covers();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Launch screen: shown when there is nothing to display — no local paths
|
||||
// and no configured library. A user who has already signed in and chosen
|
||||
// a folder goes straight to their images (FR-NC-1).
|
||||
@@ -1425,6 +1442,32 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// The current develop session, if the file yielded sensor data.
|
||||
let session: Rc<RefCell<Option<DevelopSession>>> = Rc::new(RefCell::new(None));
|
||||
|
||||
// TRACES: FR-PLAT-AND-5
|
||||
// The GPU tier — the first thing given back under memory pressure, and on
|
||||
// Android the only thing given back merely for going into the background.
|
||||
//
|
||||
// `try_borrow_mut` rather than `borrow_mut`, and the miss is not an error
|
||||
// worth reporting. A memory warning can land in the middle of a render, at
|
||||
// which point the slot is already borrowed and freeing its textures under
|
||||
// the code drawing with them is not something to do politely — skipping is
|
||||
// correct, because the pass that is running will have finished by the time
|
||||
// the platform asks again, and a warning that has not been acted on is
|
||||
// always followed by another one.
|
||||
{
|
||||
let session = Rc::downgrade(&session);
|
||||
memory::evict_at(memory::Tier::Gpu, move || {
|
||||
let Some(session) = session.upgrade() else {
|
||||
return;
|
||||
};
|
||||
let Ok(mut slot) = session.try_borrow_mut() else {
|
||||
return;
|
||||
};
|
||||
if let Some(open) = slot.as_mut() {
|
||||
open.release_gpu_caches();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-6 | FR-CAT-8
|
||||
// The settings clipboard, and where the open image's edit is stored.
|
||||
//
|
||||
|
||||
@@ -0,0 +1,276 @@
|
||||
//! TRACES: FR-PLAT-AND-5 | NFR-RES-1 | FR-NC-6b
|
||||
//! Giving memory back when the platform asks for it.
|
||||
//!
|
||||
//! Android kills the process that will not shrink. It does not negotiate and
|
||||
//! it does not warn twice, and the app it kills is the one holding the most —
|
||||
//! which, on a photo editor, is always this one. So the question this module
|
||||
//! answers is not "how much can be freed" but "in what order", because the
|
||||
//! caches differ enormously in what losing them costs.
|
||||
//!
|
||||
//! # The order, and why it is that order
|
||||
//!
|
||||
//! FR-PLAT-AND-5 states it: GPU tiles first, then proxies, then thumbnails.
|
||||
//! Read as a rule rather than a list, it is *cheapest to rebuild goes first* —
|
||||
//! a GPU allocation is remade from data already in memory, a proxy is remade
|
||||
//! from a file already on disk, and a thumbnail may cost a network fetch.
|
||||
//! [`Tier`] is that order written down where the code can be held to it, so
|
||||
//! adding a cache means choosing its tier rather than choosing its position in
|
||||
//! a hand-maintained sequence.
|
||||
//!
|
||||
//! What each tier actually reaches in this build is documented on the variant,
|
||||
//! including where it reaches nothing yet. An empty tier is worth keeping
|
||||
//! visible: it says the order is complete and the coverage is not.
|
||||
//!
|
||||
//! # Why this is a registry rather than a function that frees things
|
||||
//!
|
||||
//! Every cache worth evicting lives behind an `Rc<RefCell<…>>` owned by a
|
||||
//! local in [`crate::run`], which is a two-thousand-line function whose
|
||||
//! callbacks each hold their own handle. There is no central object to reach
|
||||
//! them through, and inventing one to serve eviction alone would be a large
|
||||
//! change to how the interface is wired for a small change in what it does.
|
||||
//!
|
||||
//! So `run` hands this module a closure per cache as it builds each one, and
|
||||
//! this module owns only the ordering. The registration is next to the thing
|
||||
//! being registered, which is also the property that keeps it honest: a cache
|
||||
//! added later is one line away from being evictable, and a cache removed
|
||||
//! takes its sink with it.
|
||||
//!
|
||||
//! # Everything here is single-threaded, and that is not a limitation
|
||||
//!
|
||||
//! The registry is a `thread_local`, holding `Fn()` rather than `Fn() + Send`,
|
||||
//! because the pressure signal already arrives on the thread that owns the
|
||||
//! caches. Slint's Android backend calls the event listener from inside
|
||||
//! `poll_events`, which runs on the same thread as the event loop, which is
|
||||
//! the thread `run` built everything on. Marshalling through
|
||||
//! `invoke_from_event_loop` would add a hop and a lifetime question to solve a
|
||||
//! problem that does not exist — and would arrive *after* the moment the
|
||||
//! system asked, which for a memory warning is the one thing that matters.
|
||||
//!
|
||||
//! Anything reached from a worker thread — the thumbnail store, the original
|
||||
//! cache — is on disk and bounded by its own budget (NFR-RES-4), and is not
|
||||
//! what a memory warning is about.
|
||||
|
||||
use std::cell::RefCell;
|
||||
|
||||
/// How hard the platform is asking.
|
||||
///
|
||||
/// Two levels rather than Android's eight, because two is what the platform
|
||||
/// actually delivers to this app. `ComponentCallbacks2.onTrimMemory` and its
|
||||
/// `TRIM_MEMORY_*` grades are a Java callback on an `Activity` or
|
||||
/// `Application`; a `NativeActivity` receives only `ANativeActivityCallbacks`,
|
||||
/// whose memory callback is the ungraded `onLowMemory` — which is what
|
||||
/// android-activity surfaces as `MainEvent::LowMemory`. Modelling grades the
|
||||
/// entry point cannot observe would be modelling a wish.
|
||||
///
|
||||
/// [`Self::UiHidden`] recovers the one distinction that *is* observable and is
|
||||
/// worth acting on, because it is the cheapest moment to give memory back:
|
||||
/// nothing is on screen, so nothing that is freed has to be drawn again before
|
||||
/// the user notices. It corresponds to `TRIM_MEMORY_UI_HIDDEN` in intent and
|
||||
/// is derived from the activity being stopped rather than from a memory
|
||||
/// warning at all.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Level {
|
||||
/// The app is no longer on screen. Free what only a visible window needs.
|
||||
UiHidden,
|
||||
/// The system says it is short of memory. Free everything that can be
|
||||
/// rebuilt.
|
||||
Critical,
|
||||
}
|
||||
|
||||
/// What a cache costs to lose, as an order.
|
||||
///
|
||||
/// Declared in eviction order and iterated in declaration order by
|
||||
/// [`Tier::ORDER`], so the sequence FR-PLAT-AND-5 specifies is a property of
|
||||
/// this type rather than of each call site.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Tier {
|
||||
/// GPU allocations that are rebuilt from data the process still holds.
|
||||
///
|
||||
/// The develop session's compiled pipelines, its detail intermediates and
|
||||
/// its output textures. Rebuilt by the next render from the demosaiced
|
||||
/// source, which is still resident — see
|
||||
/// [`DevelopSession::release_gpu_caches`](crate::DevelopSession::release_gpu_caches)
|
||||
/// for what is deliberately kept and what that is waiting on.
|
||||
///
|
||||
/// First because it is both the largest evictable pool on a mobile GPU and
|
||||
/// the cheapest to refill: no I/O, no network, one frame's work.
|
||||
Gpu,
|
||||
/// Decoded image data rebuilt by reading a file again.
|
||||
///
|
||||
/// **Nothing registers here in this build, and the tier is kept anyway.**
|
||||
/// There is no in-memory proxy cache: the only decoded full-size frame in
|
||||
/// the process is the open develop session's, which belongs to
|
||||
/// [`Tier::Gpu`] and cannot be dropped until a session can be rebuilt from
|
||||
/// a durable record (FR-PLAT-AND-3). The on-disk original cache is a
|
||||
/// different thing wearing the same word — freeing disk relieves no memory
|
||||
/// pressure, and it already has a budget and an LRU of its own
|
||||
/// (`dr_catalog::Cache`, NFR-RES-4).
|
||||
Proxies,
|
||||
/// Decoded thumbnails, rebuilt by decoding a stored JPEG again — or, at
|
||||
/// worst, by fetching one.
|
||||
///
|
||||
/// Last because this is the tier a user sees losing: an evicted portrait
|
||||
/// is a rail that redraws, and an evicted grid cell is a photograph that
|
||||
/// greys out and comes back.
|
||||
Thumbnails,
|
||||
}
|
||||
|
||||
impl Tier {
|
||||
/// The eviction order, in one place.
|
||||
pub const ORDER: [Tier; 3] = [Tier::Gpu, Tier::Proxies, Tier::Thumbnails];
|
||||
|
||||
/// Whether this tier is given up at this level of pressure.
|
||||
///
|
||||
/// Hiding the window frees the GPU tier and nothing else. That is not
|
||||
/// caution about the rest — it is that a backgrounded app has no window to
|
||||
/// draw and therefore no use at all for a render pipeline, while its
|
||||
/// thumbnails are exactly what the user will be looking at half a second
|
||||
/// after they come back. Under [`Level::Critical`] the process is being
|
||||
/// measured against being killed, and a slow return beats no return.
|
||||
fn evicted_at(self, level: Level) -> bool {
|
||||
match level {
|
||||
Level::UiHidden => matches!(self, Tier::Gpu),
|
||||
Level::Critical => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A cache that has offered itself up, and the tier it goes in.
|
||||
///
|
||||
/// Named because the registry is a `Vec` of these and the nested type is hard
|
||||
/// to read at the use site rather than because either half means anything on
|
||||
/// its own.
|
||||
type Sink = (Tier, Box<dyn Fn()>);
|
||||
|
||||
thread_local! {
|
||||
/// Registered sinks, in the order they were registered within a tier.
|
||||
///
|
||||
/// Within a tier the order is registration order and nothing depends on
|
||||
/// it; between tiers it is [`Tier::ORDER`], which everything depends on.
|
||||
static SINKS: RefCell<Vec<Sink>> = const { RefCell::new(Vec::new()) };
|
||||
}
|
||||
|
||||
/// Offer a cache up for eviction at `tier`.
|
||||
///
|
||||
/// Called as each cache is built, so that the registration reads next to the
|
||||
/// thing it is about. The closure is kept for the life of the thread; it must
|
||||
/// therefore hold weak or shared handles rather than borrow anything, which is
|
||||
/// the natural shape here because everything it can reach is already an `Rc`.
|
||||
pub(crate) fn evict_at(tier: Tier, sink: impl Fn() + 'static) {
|
||||
SINKS.with_borrow_mut(|sinks| sinks.push((tier, Box::new(sink))));
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5
|
||||
/// Give memory back, in [`Tier::ORDER`], as far down as `level` calls for.
|
||||
///
|
||||
/// Safe to call when nothing is registered — before the window is built, or on
|
||||
/// a platform that never asks — in which case it does nothing at all.
|
||||
///
|
||||
/// The registry is taken out of the cell for the duration rather than borrowed
|
||||
/// across the calls. A sink runs arbitrary interface code, and interface code
|
||||
/// that registered another cache, or called this again, would otherwise meet a
|
||||
/// `RefCell` it had already borrowed and abort the process. Freeing memory is
|
||||
/// the wrong moment to be brittle about re-entry.
|
||||
pub fn relieve(level: Level) {
|
||||
let taken: Vec<(Tier, Box<dyn Fn()>)> = SINKS.with_borrow_mut(std::mem::take);
|
||||
let mut run = 0usize;
|
||||
for tier in Tier::ORDER {
|
||||
if !tier.evicted_at(level) {
|
||||
continue;
|
||||
}
|
||||
for (t, sink) in &taken {
|
||||
if *t == tier {
|
||||
sink();
|
||||
run += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
// Put them back, keeping anything a sink registered while it ran — after,
|
||||
// so the order within a tier stays registration order.
|
||||
SINKS.with_borrow_mut(|sinks| {
|
||||
let added = std::mem::replace(sinks, taken);
|
||||
sinks.extend(added);
|
||||
});
|
||||
log::info!("memory pressure ({level:?}): ran {run} eviction(s)");
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::rc::Rc;
|
||||
|
||||
/// Registers one sink per tier, backwards, and hands back what they saw.
|
||||
fn recorder() -> Rc<RefCell<Vec<Tier>>> {
|
||||
let seen = Rc::new(RefCell::new(Vec::new()));
|
||||
for tier in [Tier::Thumbnails, Tier::Proxies, Tier::Gpu] {
|
||||
let seen = seen.clone();
|
||||
evict_at(tier, move || seen.borrow_mut().push(tier));
|
||||
}
|
||||
seen
|
||||
}
|
||||
|
||||
fn reset() {
|
||||
SINKS.with_borrow_mut(|s| s.clear());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eviction_runs_cheapest_to_rebuild_first() {
|
||||
// Registered deliberately backwards, because the guarantee is about
|
||||
// the tier and not about who registered first. A handler that simply
|
||||
// ran its list would pass every other assertion here and fail this
|
||||
// one — and on a device it would throw away thumbnails to keep a
|
||||
// render pipeline that nothing was going to draw.
|
||||
reset();
|
||||
let seen = recorder();
|
||||
relieve(Level::Critical);
|
||||
assert_eq!(
|
||||
*seen.borrow(),
|
||||
vec![Tier::Gpu, Tier::Proxies, Tier::Thumbnails]
|
||||
);
|
||||
reset();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hiding_the_window_costs_only_the_gpu() {
|
||||
// The cheap moment: give back what a window that is not on screen
|
||||
// cannot use, and keep what the user will be looking at when they come
|
||||
// back. Widening this to everything would make every task switch a
|
||||
// reload of the grid.
|
||||
reset();
|
||||
let seen = recorder();
|
||||
relieve(Level::UiHidden);
|
||||
assert_eq!(*seen.borrow(), vec![Tier::Gpu]);
|
||||
reset();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pressure_before_anything_is_registered_is_not_a_failure() {
|
||||
// The launch window: `android_main` installs the listener before
|
||||
// `run` builds a single cache, so the first minutes of a cold start
|
||||
// can deliver a warning to an empty registry.
|
||||
reset();
|
||||
relieve(Level::Critical);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_sink_may_register_another_without_deadlocking() {
|
||||
// Guards the re-entry the take-and-restore exists for: a sink is
|
||||
// interface code, and interface code that reached this module again
|
||||
// would otherwise meet a borrow it already held.
|
||||
reset();
|
||||
let seen = Rc::new(RefCell::new(0usize));
|
||||
{
|
||||
let seen = seen.clone();
|
||||
evict_at(Tier::Gpu, move || {
|
||||
*seen.borrow_mut() += 1;
|
||||
evict_at(Tier::Thumbnails, || {});
|
||||
});
|
||||
}
|
||||
relieve(Level::Critical);
|
||||
assert_eq!(*seen.borrow(), 1);
|
||||
// And the one it added survived, rather than being dropped with the
|
||||
// temporary list.
|
||||
assert_eq!(SINKS.with_borrow(|sinks| sinks.len()), 2);
|
||||
reset();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user