From 8649c75206c9337a862be9672b3d52ce0e05f73b Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Tue, 5 May 2026 17:33:22 -0700 Subject: [PATCH] perf: mmap libapp.so out of the APK instead of buffering in RAM (#354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf: mmap libapp.so out of the APK instead of buffering in RAM `open_base_lib` previously read the entire decompressed libapp.so into a Vec via `read_to_end` and handed bipatch a Cursor over that buffer. For large apps libapp.so can be tens of megabytes, and that allocation happens immediately after the patch download is buffered to disk — a plausible OOM trigger on memory-constrained devices (we have at least one customer report on OnePlus where the patch install appears to halt silently right after the download completes). Modern AGP (3.6+) defaults to extractNativeLibs=false, which keeps libapp.so STORED uncompressed inside the APK so the dynamic linker can mmap it directly. When that's the case, do the same: find the entry's data offset via the zip crate, drop the archive, reopen the APK, and mmap the entry's slice. Cursor implements Read + Seek, which is what bipatch's `Reader::new(patch, base)` requires. When the entry isn't stored uncompressed (older builds, or builds that explicitly compress native libs), fall back to the previous buffered read so we always succeed. Mmap doesn't change the peak working set when bipatch traverses the whole base linearly, but file-backed mappings are clean and reclaimable under memory pressure where an anonymous Vec is not, and we avoid the ~2x transient allocation peak from `read_to_end` growing the buffer. Tests cover both paths (stored → mmap, deflated → buffered) on the host. * ci: add mmap, memmap, SIGBUS to cspell dictionary --- Cargo.lock | 10 +++ cspell.config.yaml | 3 + library/Cargo.toml | 4 ++ library/src/android.rs | 147 ++++++++++++++++++++++++++++++++++++++--- library/src/updater.rs | 3 +- 5 files changed, 157 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebfcda3..733c1ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -982,6 +982,15 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "memmap2" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "714098028fe011992e1c3962653c96b2d578c4b4bce9036e15ff220319b1e0e3" +dependencies = [ + "libc", +] + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -1852,6 +1861,7 @@ dependencies = [ "libc", "log", "log-panics", + "memmap2", "mock_instant", "mockall", "mockito", diff --git a/cspell.config.yaml b/cspell.config.yaml index 52f1174..8a6484e 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -55,6 +55,8 @@ words: - libflutter - libupdater - logcat + - memmap + - mmap - mockall - mocktail - msvc @@ -72,6 +74,7 @@ words: - rustup - serde - shorebirdtech + - SIGBUS - sigstore - staticlib - subosito diff --git a/library/Cargo.toml b/library/Cargo.toml index 5031787..3205cda 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -36,6 +36,10 @@ hex = "0.4.3" libc = "0.2.98" # For error!(), info!(), etc macros. `print` will not show up on Android. log = "0.4.14" +# For mmap'ing libapp.so out of the APK without buffering it in RAM. This +# matters on memory-constrained devices: reading the whole library into a +# Vec can cost tens of MB and trigger OOM kills mid-patch. +memmap2 = "0.9.10" # For implementing thread-local-storage of ResolvedConfig object. once_cell = "1.17.1" # Json serialization/de-serialization. diff --git a/library/src/android.rs b/library/src/android.rs index 7a6bdca..eb26e9b 100644 --- a/library/src/android.rs +++ b/library/src/android.rs @@ -1,10 +1,13 @@ // cspell:ignore rpkDZSLBRv2jWcc1gQpwdg use anyhow::Context; +use memmap2::{Mmap, MmapOptions}; use std::fs; use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; -use crate::InitError; +use crate::{InitError, ReadSeek}; + +impl ReadSeek for Cursor {} /// This function is a hack for Android. Android passes an array of paths, the /// first of which is `libapp.so` the second of which is a long (virtual) path @@ -84,10 +87,14 @@ pub(crate) fn get_relative_lib_path(lib_name: &str) -> PathBuf { // Ideally we'd just return the ZipFile itself, but I don't know how to set // up the references correctly, ZipFile contains a borrow into the ZipArchive. // And I'm not the right Rust to keep a reference to both with proper lifetimes. +// +// We also keep the path to the APK on disk so we can re-open it for mmap +// in `open_base_lib` without re-walking the apks_dir. #[derive(Debug)] struct ZipLocation { archive: zip::ZipArchive, internal_path: String, + zip_path: PathBuf, } /// Given a zip file, check if it contains the library we want. @@ -97,6 +104,7 @@ fn check_for_lib_path(zip_path: &Path, lib_path: &str) -> anyhow::Result anyhow::Result anyhow::Result>> { +pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result> { // As far as I can tell, Android provides no apis for reading per-platform // assets (e.g. libapp.so) from an APK. Both Facebook and Chromium // seem to have written their own code to do this: @@ -172,13 +180,64 @@ pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result allocation right after a patch download is a + // plausible OOM trigger on memory-constrained devices. + if zip_file.compression() == zip::CompressionMethod::Stored { + if let Some(data_start) = zip_file.data_start() { + let len = zip_file.size(); + // Drop the ZipFile reader so we release the borrow on the + // archive and on its underlying File handle before we re-open + // the APK for mapping. + drop(zip_file); + return mmap_zip_entry(&zip_location.zip_path, data_start, len); + } + } + + // Fallback: the entry is compressed (or the zip crate didn't surface a + // data offset for some reason). Decompress fully into RAM. + shorebird_debug!("libapp.so is not stored uncompressed; falling back to buffered read"); let mut buffer = Vec::new(); zip_file.read_to_end(&mut buffer)?; - Ok(Cursor::new(buffer)) + Ok(Box::new(Cursor::new(buffer))) +} + +/// Open `zip_path` and mmap a `len`-byte region starting at `data_start`. +/// +/// Safety: the mapping aliases the file's contents; concurrent writes by +/// another process would produce torn reads or SIGBUS. APKs on Android are +/// installed read-only and not modified at runtime, so this is sound for +/// our use. We also drop the File handle after mapping — the kernel keeps +/// the mapping alive independently. +fn mmap_zip_entry(zip_path: &Path, data_start: u64, len: u64) -> anyhow::Result> { + let len_usize = usize::try_from(len) + .with_context(|| format!("entry size {} does not fit in usize", len))?; + let file = fs::File::open(zip_path) + .with_context(|| format!("Failed to reopen APK for mmap: {:?}", zip_path))?; + // SAFETY: see function-level comment. + let mmap = unsafe { + MmapOptions::new() + .offset(data_start) + .len(len_usize) + .map(&file) + .with_context(|| { + format!( + "Failed to mmap {:?} at offset {} for {} bytes", + zip_path, data_start, len + ) + })? + }; + shorebird_debug!( + "Mapped libapp.so via mmap: {} bytes at offset {} of {:?}", + len, + data_start, + zip_path + ); + Ok(Box::new(Cursor::new(mmap))) } pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result { @@ -210,9 +269,11 @@ pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result isn't Debug, so unwrap_err() doesn't apply. + let error = super::open_base_lib(tmp_dir.path(), "libapp.so") + .err() + .expect("expected open_base_lib to fail with no APKs present"); assert_error_is_file_not_found(&error); } + + /// Build an APK in `apk_path` containing a single entry whose path is + /// `lib//` and whose contents are `data`, written with + /// the given compression method. + fn write_apk_with_lib( + apk_path: &Path, + lib_name: &str, + data: &[u8], + compression: CompressionMethod, + ) { + let arch = super::android_arch_names(); + let internal_path = Path::new("lib").join(arch.lib_dir).join(lib_name); + let file = File::create(apk_path).unwrap(); + let mut zip = ZipWriter::new(file); + zip.start_file( + internal_path.to_string_lossy(), + SimpleFileOptions::default().compression_method(compression), + ) + .unwrap(); + zip.write_all(data).unwrap(); + zip.finish().unwrap(); + } + + /// When the entry is STORED uncompressed (the modern AGP default), we + /// should mmap it and the returned reader should yield the original + /// bytes via Read + Seek. + #[test] + fn open_base_lib_stored_uses_mmap_and_reads_bytes() { + let tmp_dir = TempDir::new().unwrap(); + let apk_path = tmp_dir.path().join("base.apk"); + let payload: Vec = (0..2048u32).map(|i| (i % 251) as u8).collect(); + write_apk_with_lib(&apk_path, "libapp.so", &payload, CompressionMethod::Stored); + + let mut reader = super::open_base_lib(tmp_dir.path(), "libapp.so").unwrap(); + + // Full read. + let mut buf = Vec::new(); + reader.read_to_end(&mut buf).unwrap(); + assert_eq!(buf, payload); + + // Seek + partial read — proves the returned reader satisfies + // Read + Seek (which is what bipatch needs). + reader.seek(SeekFrom::Start(100)).unwrap(); + let mut chunk = vec![0u8; 16]; + reader.read_exact(&mut chunk).unwrap(); + assert_eq!(chunk, payload[100..116]); + } + + /// When the entry is DEFLATEd, we fall back to a buffered read. The + /// returned reader should still produce the uncompressed bytes. + #[test] + fn open_base_lib_deflated_falls_back_to_buffered_read() { + let tmp_dir = TempDir::new().unwrap(); + let apk_path = tmp_dir.path().join("base.apk"); + let payload: Vec = (0..1024u32).map(|i| (i % 17) as u8).collect(); + write_apk_with_lib( + &apk_path, + "libapp.so", + &payload, + CompressionMethod::Deflated, + ); + + let mut reader = super::open_base_lib(tmp_dir.path(), "libapp.so").unwrap(); + let mut buf = Vec::new(); + reader.read_to_end(&mut buf).unwrap(); + assert_eq!(buf, payload); + } } diff --git a/library/src/updater.rs b/library/src/updater.rs index fb0ca86..908e570 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -344,8 +344,7 @@ impl ReadSeek for fs::File {} // FIXME: these patch_base functions should move to platform-specific modules where they can all be tested. #[cfg(any(target_os = "android", test))] fn patch_base(config: &UpdateConfig) -> anyhow::Result> { - let base_r = crate::android::open_base_lib(&config.libapp_path, "libapp.so")?; - Ok(Box::new(base_r)) + crate::android::open_base_lib(&config.libapp_path, "libapp.so") } #[cfg(target_os = "ios")]