Skip to content

Commit

Permalink
Fix #2229 detect available memory
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rbtcollins committed Feb 27, 2020
1 parent bd2d51e commit 7fb9d3f
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 57 deletions.
99 changes: 64 additions & 35 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
48 changes: 38 additions & 10 deletions src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}))?)
}

Expand Down
38 changes: 28 additions & 10 deletions src/dist/component/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 11 additions & 2 deletions src/utils/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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.
Expand Down Expand Up @@ -54,7 +58,7 @@ impl<'a> Notification<'a> {
| ResumingPartialDownload
| UsingCurl
| UsingReqwest => NotificationLevel::Verbose,
RenameInUse(_, _) => NotificationLevel::Info,
RenameInUse(_, _) | SetDefaultBufferSize(_) => NotificationLevel::Info,
NoCanonicalPath(_) => NotificationLevel::Warn,
}
}
Expand All @@ -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()),
Expand Down
3 changes: 3 additions & 0 deletions tests/cli-exact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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
),
Expand Down
5 changes: 5 additions & 0 deletions tests/cli-rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions tests/cli-self-upd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions tests/cli-v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
"
Expand Down

0 comments on commit 7fb9d3f

Please sign in to comment.