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.

Author: rbtcollins

Cargo.lock

@@ -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"

Cargo.toml

@@ -5,6 +5,8 @@ use crate::self_update;
 use crate::term2;
 use git_testament::{git_testament, render_testament};
 use lazy_static::lazy_static;
+use rustup::dist::notifications as dist_notifications;
+use rustup::utils::notifications as util_notifications;
 use rustup::utils::notify::NotificationLevel;
 use rustup::utils::utils;
 use rustup::{Cfg, Notification, Toolchain, UpdateStatus};
@@ -104,21 +106,29 @@ pub fn read_line() -> Result<String> {
         .ok_or_else(|| "unable to read from stdin for confirmation".into())
 }
 
-pub fn set_globals(verbose: bool, quiet: bool) -> Result<Cfg> {
-    use crate::download_tracker::DownloadTracker;
-    use std::cell::RefCell;
-
-    let download_tracker = RefCell::new(DownloadTracker::new().with_display_progress(!quiet));
+#[derive(Default)]
+struct NotifyOnConsole {
+    ram_notice_shown: bool,
+    verbose: bool,
+}
 
-    Ok(Cfg::from_env(Arc::new(move |n: Notification<'_>| {
-        if download_tracker.borrow_mut().handle_notification(&n) {
-            return;
-        }
+impl NotifyOnConsole {
+    fn handle(&mut self, n: Notification<'_>) {
+        if let Notification::Install(dist_notifications::Notification::Utils(
+            util_notifications::Notification::SetDefaultBufferSize(_),
+        )) = &n
+        {
+            if self.ram_notice_shown {
+                return;
+            } else {
+                self.ram_notice_shown = true;
+            }
+        };
         let level = n.level();
         for n in format!("{}", n).lines() {
             match level {
                 NotificationLevel::Verbose => {
-                    if verbose {
+                    if self.verbose {
                         verbose!("{}", n);
                     }
                 }
@@ -133,6 +143,24 @@ pub fn set_globals(verbose: bool, quiet: bool) -> Result<Cfg> {
                 }
             }
         }
+    }
+}
+
+pub fn set_globals(verbose: bool, quiet: bool) -> Result<Cfg> {
+    use crate::download_tracker::DownloadTracker;
+    use std::cell::RefCell;
+
+    let download_tracker = RefCell::new(DownloadTracker::new().with_display_progress(!quiet));
+    let console_notifier = RefCell::new(NotifyOnConsole {
+        verbose,
+        ..Default::default()
+    });
+
+    Ok(Cfg::from_env(Arc::new(move |n: Notification<'_>| {
+        if download_tracker.borrow_mut().handle_notification(&n) {
+            return;
+        }
+        console_notifier.borrow_mut().handle(n);
     }))?)
 }
 

src/cli/common.rs

@@ -165,17 +165,30 @@ struct MemoryBudget {
 
 // 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,
+        notify_handler: Option<&dyn Fn(Notification<'_>)>,
+    ) -> 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 let Some(h) = notify_handler {
+                    h(Notification::SetDefaultBufferSize(default_max_unpack_ram))
+                }
+                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 +291,12 @@ 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,
+        notify_handler,
+    );
 
     let mut directories: HashMap<PathBuf, DirStatus> = HashMap::new();
     // Path is presumed to exist. Call it a precondition.

src/dist/component/package.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 {

src/errors.rs

@@ -4,7 +4,7 @@ use std::path::Path;
 use url::Url;
 
 use crate::utils::notify::NotificationLevel;
-use crate::utils::units::Unit;
+use crate::utils::units::{self, Unit};
 
 #[derive(Debug)]
 pub enum Notification<'a> {
@@ -27,6 +27,10 @@ pub enum Notification<'a> {
     DownloadPopUnit,
     NoCanonicalPath(&'a Path),
     ResumingPartialDownload,
+    /// This would make more sense as a crate::notifications::Notification
+    /// member, but the notification callback is already narrowed to
+    /// utils::notifications by the time tar unpacking is called.
+    SetDefaultBufferSize(usize),
     UsingCurl,
     UsingReqwest,
     /// Renaming encountered a file in use error and is retrying.
@@ -54,7 +58,7 @@ impl<'a> Notification<'a> {
             | ResumingPartialDownload
             | UsingCurl
             | UsingReqwest => NotificationLevel::Verbose,
-            RenameInUse(_, _) => NotificationLevel::Info,
+            RenameInUse(_, _) | SetDefaultBufferSize(_) => NotificationLevel::Info,
             NoCanonicalPath(_) => NotificationLevel::Warn,
         }
     }
@@ -78,6 +82,11 @@ impl<'a> Display for Notification<'a> {
                 src.display(),
                 dest.display()
             ),
+            SetDefaultBufferSize(size) => write!(
+                f,
+                "Defaulting to {} unpack ram",
+                units::Size::new(*size, units::Unit::B, units::UnitMode::Norm)
+            ),
             DownloadingFile(url, _) => write!(f, "downloading file from: '{}'", url),
             DownloadContentLengthReceived(len) => write!(f, "download size is: '{}'", len),
             DownloadDataReceived(data) => write!(f, "received some data of size {}", data.len()),

src/utils/notifications.rs

@@ -38,6 +38,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
+info: 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'
+info: 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}'
+info: Defaulting to 500.0 MiB unpack ram
 ",
                 clitools::CROSS_ARCH1
             ),

tests/cli-exact.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'
+info: 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'
+info: 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'
+info: 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'
+info: 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'
+info: Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'

tests/cli-rustup.rs

@@ -883,6 +883,7 @@ info: downloading component 'rust-docs'
 info: downloading component 'rust-std'
 info: downloading component 'rustc'
 info: installing component 'cargo'
+info: Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rust-std'
 info: installing component 'rustc'

tests/cli-self-upd.rs

@@ -1342,6 +1342,7 @@ info: downloading component 'cargo'
 info: downloading component 'rust-docs'
 info: downloading component 'rustc'
 info: installing component 'cargo'
+info: Defaulting to 500.0 MiB unpack ram
 info: installing component 'rust-docs'
 info: installing component 'rustc'
 "