Merge pull request #2236 from rbtcollins/2229

Fix #2229 detect available memory

Author: kinnison

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