Fix #2229 detect available memory

This doesn't implement streaming IO for low memory situations - we still have the situation that low footprint situations will fail to install, but while it is the case that rustc's memory footprint is lower than our unpack footprint this is probably not urgent to fix, though I will get around to it. Being less aggressive about unpack buffer size though should reduce the number of support tickets from folk in these cases, I hope. We may end up getting tickets from folk with broken ulimit syscalls though, who knows.
rust-lang · Feb 26, 2020 · 719ace8 · 719ace8
1 parent bd2d51e
commit 719ace8
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 45 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -25,6  25,7 @@ chrono = "0.4"
 clap = "2"
 download = { path = "download" }
 error-chain = "0.12"
 effective-limits = "0.3"
 flate2 = "1"
 git-testament = "0.1.4"
 home = "0.5"

diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs
@@ -8,6  8,7 @@ use crate::dist::component::transaction::*;
 use crate::dist::temp;
 use crate::errors::*;
 use crate::utils::notifications::Notification;
 use crate::utils::units;
 use crate::utils::utils;
 
 use std::collections::{HashMap, HashSet};
@@ -17,7  18,9 @@ use std::io::{self, ErrorKind as IOErrorKind, Read};
 use std::iter::FromIterator;
 use std::mem;
 use std::path::{Path, PathBuf};
 use std::sync::atomic::{AtomicBool, Ordering};
 
 use lazy_static::lazy_static;
 use tar::EntryType;
 
 /// The current metadata revision used by rust-installer
@@ -163,19  166,40 @@ struct MemoryBudget {
     used: usize,
 }
 
 lazy_static! {
     static ref RAM_NOTICE_SHOWN: AtomicBool = AtomicBool::new(false);
 }
 
 // Probably this should live in diskio but ¯\_(ツ)_/¯
 impl MemoryBudget {
-    fn new(max_file_size: usize) -> Self {
-        const DEFAULT_UNPACK_RAM: usize = 400 * 1024 * 1024;
-        let unpack_ram = if let Ok(budget_str) = env::var("RUSTUP_UNPACK_RAM") {
-            if let Ok(budget) = budget_str.parse::<usize>() {
-                budget
-            } else {
-                DEFAULT_UNPACK_RAM
     fn new(max_file_size: usize, effective_max_ram: usize) -> Self {
         const DEFAULT_UNPACK_RAM_MAX: usize = 500 * 1024 * 1024;
         const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 100 * 1024 * 1024;
         let ram_for_unpacking = effective_max_ram - RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS;
         let default_max_unpack_ram = std::cmp::min(DEFAULT_UNPACK_RAM_MAX, ram_for_unpacking);
         let unpack_ram = match env::var("RUSTUP_UNPACK_RAM")
             .ok()
             .and_then(|budget_str| budget_str.parse::<usize>().ok())
         {
             // Note: In future we may want to add a warning or even an override if a user
             // supplied budget is larger than effective_max_ram.
             Some(budget) => budget,
             None => {
                 if RAM_NOTICE_SHOWN.load(Ordering::Acquire) {
                     eprintln!(
                         "Defaulting to {} unpack ram",
                         units::Size::new(
                             default_max_unpack_ram,
                             units::Unit::B,
                             units::UnitMode::Norm
                         )
                     );
                     RAM_NOTICE_SHOWN.store(true, Ordering::Release);
                 }
                 default_max_unpack_ram
             }
-        } else {
-            DEFAULT_UNPACK_RAM
         };
 
         if max_file_size > unpack_ram {
             panic!("RUSTUP_UNPACK_RAM must be larger than {}", max_file_size);
         }
@@ -278,7  302,8 @@ fn unpack_without_first_dir<'a, R: Read>(
         .entries()
         .chain_err(|| ErrorKind::ExtractingPackage)?;
     const MAX_FILE_SIZE: u64 = 200_000_000;
-    let mut budget = MemoryBudget::new(MAX_FILE_SIZE as usize);
     let effective_max_ram = effective_limits::memory_limit()?;
     let mut budget = MemoryBudget::new(MAX_FILE_SIZE as usize, effective_max_ram as usize);
 
     let mut directories: HashMap<PathBuf, DirStatus> = HashMap::new();
     // Path is presumed to exist. Call it a precondition.

diff --git a/src/errors.rs b/src/errors.rs
@@ -22,6  22,7 @@ pub const TOOLSTATE_MSG: &str =
 error_chain! {
     links {
         Download(download::Error, download::ErrorKind);
         Limits(effective_limits::Error, effective_limits::ErrorKind);
     }
 
     foreign_links {

diff --git a/tests/cli-exact.rs b/tests/cli-exact.rs
@@ -38,6  38,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -172,6  173,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -504,6  506,7 @@ fn cross_install_indicates_target() {
             &format!(
                 r"info: downloading component 'rust-std' for '{0}'
 info: installing component 'rust-std' for '{0}'
 Defaulting to 500.0 MiB unpack ram
 ",
                 clitools::CROSS_ARCH1
             ),

diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs
@@ -56,6  56,7 @@ info: removing previous version of component 'rust-docs'
 info: removing previous version of component 'rust-std'
 info: removing previous version of component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -96,6  97,7 @@ info: removing previous version of component 'rust-docs'
 info: removing previous version of component 'rust-std'
 info: removing previous version of component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -160,6  162,7 @@ info: removing previous version of component 'rust-docs'
 info: removing previous version of component 'rust-std'
 info: removing previous version of component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -230,6  233,7 @@ info: removing previous version of component 'rust-docs'
 info: removing previous version of component 'rust-std'
 info: removing previous version of component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'
@@ -291,6  295,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'

diff --git a/tests/cli-self-upd.rs b/tests/cli-self-upd.rs
@@ -883,6  883,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'

diff --git a/tests/cli-v2.rs b/tests/cli-v2.rs
@@ -1342,6  1342,7 @@ info: downloading component 'cargo'
 info: downloading component 'rust-docs'
 info: downloading component 'rustc'
 info: installing component 'cargo'
 Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rustc'
 "