diff --git a/apps/photocraft-cli/src/main.rs b/apps/photocraft-cli/src/main.rs index ea5598f99d..6e21c2ac44 100644 --- a/apps/photocraft-cli/src/main.rs +++ b/apps/photocraft-cli/src/main.rs @@ -1,6 +1,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] fn main() { + // May re-execute this process with glibc tuned to return freed document memory (#8743). + photocraft_engine::malloc_tuning::apply(); // `args_os`, not `args`: a non-Unicode argument (a Latin-1 file name on Linux) must reach clap, // not panic before anything is printed (issue #1108). let code = photocraft_cli::run(std::env::args_os().skip(1), &mut std::io::stdout(), &mut std::io::stderr()); diff --git a/apps/photocraft-cli/tests/it/main.rs b/apps/photocraft-cli/tests/it/main.rs index 65092cdee4..2729f74251 100644 --- a/apps/photocraft-cli/tests/it/main.rs +++ b/apps/photocraft-cli/tests/it/main.rs @@ -1,6 +1,8 @@ //! The crate's integration tests, linked into one test binary. mod cli; +#[cfg(all(target_os = "linux", target_env = "gnu"))] +mod malloc_tuning; mod man_and_completions; mod orientation; diff --git a/apps/photocraft-cli/tests/it/malloc_tuning.rs b/apps/photocraft-cli/tests/it/malloc_tuning.rs new file mode 100644 index 0000000000..a7513a714a --- /dev/null +++ b/apps/photocraft-cli/tests/it/malloc_tuning.rs @@ -0,0 +1,41 @@ +//! The binary re-executes itself with glibc's trim threshold set (#8743), keeping its arguments +//! and a value the user set. + +use std::io::{BufRead, BufReader, Write}; +use std::process::{Command, Stdio}; + +use photocraft_engine::malloc_tuning::{TRIM_THRESHOLD, TRIM_THRESHOLD_VAR}; + +/// The environment of a running `serve` once it answered a request, as `NAME=value` entries. +fn serve_environ(set: Option<&str>) -> Vec { + let mut cmd = Command::new(crate::binary()); + for var in [TRIM_THRESHOLD_VAR, "MALLOC_MMAP_THRESHOLD_", "MALLOC_TOP_PAD_", "MALLOC_MMAP_MAX_", "MALLOC_ARENA_MAX", "GLIBC_TUNABLES"] { + cmd.env_remove(var); + } + if let Some(v) = set { + cmd.env(TRIM_THRESHOLD_VAR, v); + } + let mut child = cmd.arg("serve").stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::null()).spawn().unwrap(); + let mut stdin = child.stdin.take().unwrap(); + writeln!(stdin, r#"{{"id":1,"method":"session.list"}}"#).unwrap(); + let mut reply = String::new(); + BufReader::new(child.stdout.take().unwrap()).read_line(&mut reply).unwrap(); + assert!(reply.contains(r#""id":1"#), "serve answered after the re-exec: {reply}"); + // The same process: `exec` keeps the PID, so this is the re-executed program's environment. + let environ = std::fs::read(format!("/proc/{}/environ", child.id())).unwrap(); + drop(stdin); + child.wait().unwrap(); + environ.split(|&b| b == 0).map(|e| String::from_utf8_lossy(e).into_owned()).collect() +} + +#[test] +fn sets_the_trim_threshold() { + let env = serve_environ(None); + assert!(env.contains(&format!("{TRIM_THRESHOLD_VAR}={TRIM_THRESHOLD}")), "{env:?}"); +} + +#[test] +fn keeps_a_value_the_user_set() { + let env = serve_environ(Some("999999")); + assert!(env.contains(&format!("{TRIM_THRESHOLD_VAR}=999999")), "{env:?}"); +} diff --git a/apps/photocraft/src/main.rs b/apps/photocraft/src/main.rs index e19a9d45e8..3738ffc8d1 100644 --- a/apps/photocraft/src/main.rs +++ b/apps/photocraft/src/main.rs @@ -340,7 +340,10 @@ fn write_start_marker(sentinel: &gpu_startup::SharedSentinel, plan: &gpu_startup } fn main() -> eframe::Result { - // First, so the panic hook and every start-up warning are recorded (`logging`). + // Before anything starts a thread: may re-execute this process with glibc tuned to return + // the memory of closed documents (#8743). + photocraft_engine::malloc_tuning::apply(); + // Then, so the panic hook and every start-up warning are recorded (`logging`). let logger = logging::install(); crash_guard::install_hook(); #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5e6cc66ab0..b49675176e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -29,6 +29,7 @@ pub mod kys; pub mod layer_place; pub mod layer_style; pub mod link_watch; +pub mod malloc_tuning; pub mod new_layer; mod param_check; mod pixels; diff --git a/crates/engine/src/malloc_tuning.rs b/crates/engine/src/malloc_tuning.rs new file mode 100644 index 0000000000..f9c5947c47 --- /dev/null +++ b/crates/engine/src/malloc_tuning.rs @@ -0,0 +1,81 @@ +//! glibc malloc tuning for the native binaries (#8743). +//! +//! With glibc's defaults, the memory of a closed document stays with the process: freeing a large +//! buffer raises glibc's dynamic mmap threshold (up to 32 MiB), so later tile buffers come from the +//! heap, whose freed pages glibc keeps for reuse instead of giving them back. A session that opened +//! and closed a 5400 × 7200 document held ~540 MiB afterwards; with `MALLOC_TRIM_THRESHOLD_` set it +//! went back to ~20 MiB, and the peak was lower too. Setting the threshold explicitly turns the +//! dynamic adjustment off (mallopt(3)), so large buffers are mapped and unmapped on their own and +//! the heap top is trimmed as it is freed. Mapping fresh pages on every call is slow, so the large +//! short-lived buffers come from `photocraft_raster::scratch` instead, which reuses them and +//! releases them when a document closes or the pool goes idle (#10479). +//! +//! glibc reads its tunables once, when the process starts, and changing the environment from Rust +//! needs `unsafe` (and comes too late), so [`apply`] re-executes the binary with the variable set: +//! the same process (PID), arguments, `argv[0]` and environment, on Linux with glibc only. A value +//! the user or a package already set, for this or another malloc tunable, is left alone, so setting +//! `MALLOC_TRIM_THRESHOLD_` to anything opts out. Programs PhotoCraft starts inherit the variable. + +use std::ffi::OsString; + +/// The glibc environment variable [`apply`] sets. +pub const TRIM_THRESHOLD_VAR: &str = "MALLOC_TRIM_THRESHOLD_"; + +/// Its value: glibc's own default trim threshold (128 KiB), now fixed instead of dynamic. +pub const TRIM_THRESHOLD: &str = "131072"; + +/// glibc malloc variables that mean someone tuned malloc themselves. +const MALLOC_VARS: [&str; 5] = [TRIM_THRESHOLD_VAR, "MALLOC_MMAP_THRESHOLD_", "MALLOC_TOP_PAD_", "MALLOC_MMAP_MAX_", "MALLOC_ARENA_MAX"]; + +/// Whether a start-up should re-execute with [`TRIM_THRESHOLD_VAR`] set, given the environment: +/// only when no malloc variable or `glibc.malloc.*` tunable is set (which also ends the re-exec). +pub fn needs_restart(var: impl Fn(&str) -> Option) -> bool { + if MALLOC_VARS.iter().any(|v| var(v).is_some()) { + return false; + } + !var("GLIBC_TUNABLES").is_some_and(|t| t.to_string_lossy().contains("glibc.malloc.")) +} + +/// Re-execute this binary with [`TRIM_THRESHOLD_VAR`] set when [`needs_restart`] says so. Call it +/// first in `main`, before any thread starts. It returns only when nothing needed to change or the +/// re-exec failed, and then the process carries on with glibc's defaults. +#[cfg(all(target_os = "linux", target_env = "gnu"))] +pub fn apply() { + use std::os::unix::process::CommandExt; + if !needs_restart(|k| std::env::var_os(k)) { + return; + } + let Ok(exe) = std::env::current_exe() else { return }; + let mut args = std::env::args_os(); + let mut cmd = std::process::Command::new(exe); + if let Some(argv0) = args.next() { + cmd.arg0(argv0); + } + // `exec` replaces the process and only returns on failure. + let err = cmd.args(args).env(TRIM_THRESHOLD_VAR, TRIM_THRESHOLD).exec(); + eprintln!("PhotoCraft: could not restart with {TRIM_THRESHOLD_VAR} set ({err}); memory of closed documents may not be returned to the system"); +} + +/// Nothing to tune off glibc. +#[cfg(not(all(target_os = "linux", target_env = "gnu")))] +pub fn apply() {} + +#[cfg(test)] +mod tests { + use super::*; + + fn env<'a>(vars: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + 'a { + move |k| vars.iter().find(|(n, _)| *n == k).map(|(_, v)| OsString::from(v)) + } + + #[test] + fn restarts_only_without_malloc_tuning() { + assert!(needs_restart(env(&[]))); + assert!(needs_restart(env(&[("HOME", "/home/a"), ("GLIBC_TUNABLES", "glibc.cpu.hwcaps=-AVX2")]))); + assert!(!needs_restart(env(&[(TRIM_THRESHOLD_VAR, TRIM_THRESHOLD)])), "the re-executed process stops"); + assert!(!needs_restart(env(&[(TRIM_THRESHOLD_VAR, "")])), "any value opts out"); + assert!(!needs_restart(env(&[("MALLOC_MMAP_THRESHOLD_", "1048576")]))); + assert!(!needs_restart(env(&[("MALLOC_ARENA_MAX", "2")]))); + assert!(!needs_restart(env(&[("GLIBC_TUNABLES", "glibc.malloc.trim_threshold=0")]))); + } +}