Skip to content

Commit

Permalink
Auto merge of #6853 - Eh2406:caching-the-dependency, r=alexcrichton
Browse files Browse the repository at this point in the history
Caching the dependencies

There are 2 sources of facts for the resolver:
1. The `Registry` tells us for a Dependency what versions are available to fulfil it.
2. The `Summary` tells us for a version (and features) what dependencies need to be fulfilled for it to be activated.

The `Registry` was cached with a `RegistryQueryer` back in #5112. This adds a `DepsCache` to cache the calculation of which dependencies are activated by features.

In the happy path `flag_activated` means that we don't get to reuse `build_deps`, but the more we backtrack the more time we save. In pathological cases like #6258 (comment), I have measured this as a 10% improvement with release.

This also means that `build_deps` can be run in a context free way, which may be useful in a follow up PR to solve #6258 (comment).
  • Loading branch information
bors committed Apr 26, 2019
2 parents c9330fe + 79714ba commit 0c891a0
Show file tree
Hide file tree
Showing 14 changed files with 594 additions and 542 deletions.
15 changes: 5 additions & 10 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,7 @@ enum LocalFingerprint {
/// The `dep_info` file, when present, also lists a number of other files
/// for us to look at. If any of those files are newer than this file then
/// we need to recompile.
CheckDepInfo {
dep_info: PathBuf,
},
CheckDepInfo { dep_info: PathBuf },

/// This represents a nonempty set of `rerun-if-changed` annotations printed
/// out by a build script. The `output` file is a arelative file anchored at
Expand All @@ -500,10 +498,7 @@ enum LocalFingerprint {
/// build script. The exact env var and value are hashed here. There's no
/// filesystem dependence here, and if the values are changed the hash will
/// change forcing a recompile.
RerunIfEnvChanged {
var: String,
val: Option<String>,
},
RerunIfEnvChanged { var: String, val: Option<String> },
}

enum StaleFile {
Expand Down Expand Up @@ -762,7 +757,7 @@ impl Fingerprint {
let t = FileTime::from_system_time(SystemTime::now());
drop(filetime::set_file_times(f, t, t));
}
return mtime;
mtime
})
.min();

Expand Down Expand Up @@ -1350,7 +1345,7 @@ fn compare_old_fingerprint(
debug_assert_eq!(util::to_hex(old_fingerprint.hash()), old_fingerprint_short);
let result = new_fingerprint.compare(&old_fingerprint);
assert!(result.is_err());
return result;
result
}

fn log_compare(unit: &Unit<'_>, compare: &CargoResult<()>) {
Expand Down Expand Up @@ -1444,7 +1439,7 @@ where
});
}

return None;
None
}

fn filename<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> String {
Expand Down
40 changes: 21 additions & 19 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use semver::Version;
use serde::{de, ser};
use url::Url;

use crate::core::interning::InternedString;
use crate::core::PackageId;
use crate::util::errors::{CargoResult, CargoResultExt};
use crate::util::{validate_package_name, ToSemver, ToUrl};
Expand All @@ -20,7 +21,7 @@ use crate::util::{validate_package_name, ToSemver, ToUrl};
/// sufficient to uniquely define a package ID.
#[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)]
pub struct PackageIdSpec {
name: String,
name: InternedString,
version: Option<Version>,
url: Option<Url>,
}
Expand Down Expand Up @@ -66,7 +67,7 @@ impl PackageIdSpec {
};
validate_package_name(name, "pkgid", "")?;
Ok(PackageIdSpec {
name: name.to_string(),
name: InternedString::new(name),
version,
url: None,
})
Expand All @@ -86,7 +87,7 @@ impl PackageIdSpec {
/// fields filled in.
pub fn from_package_id(package_id: PackageId) -> PackageIdSpec {
PackageIdSpec {
name: package_id.name().to_string(),
name: package_id.name(),
version: Some(package_id.version().clone()),
url: Some(package_id.source_id().url().clone()),
}
Expand Down Expand Up @@ -117,19 +118,19 @@ impl PackageIdSpec {
match parts.next() {
Some(part) => {
let version = part.to_semver()?;
(name_or_version.to_string(), Some(version))
(InternedString::new(name_or_version), Some(version))
}
None => {
if name_or_version.chars().next().unwrap().is_alphabetic() {
(name_or_version.to_string(), None)
(InternedString::new(name_or_version), None)
} else {
let version = name_or_version.to_semver()?;
(path_name.to_string(), Some(version))
(InternedString::new(path_name), Some(version))
}
}
}
}
None => (path_name.to_string(), None),
None => (InternedString::new(path_name), None),
}
};
Ok(PackageIdSpec {
Expand All @@ -139,8 +140,8 @@ impl PackageIdSpec {
})
}

pub fn name(&self) -> &str {
&self.name
pub fn name(&self) -> InternedString {
self.name
}

pub fn version(&self) -> Option<&Version> {
Expand All @@ -157,7 +158,7 @@ impl PackageIdSpec {

/// Checks whether the given `PackageId` matches the `PackageIdSpec`.
pub fn matches(&self, package_id: PackageId) -> bool {
if self.name() != &*package_id.name() {
if self.name() != package_id.name() {
return false;
}

Expand Down Expand Up @@ -234,7 +235,7 @@ impl fmt::Display for PackageIdSpec {
} else {
write!(f, "{}", url)?;
}
if url.path_segments().unwrap().next_back().unwrap() != self.name {
if url.path_segments().unwrap().next_back().unwrap() != &*self.name {
printed_name = true;
write!(f, "#{}", self.name)?;
}
Expand Down Expand Up @@ -273,6 +274,7 @@ impl<'de> de::Deserialize<'de> for PackageIdSpec {
#[cfg(test)]
mod tests {
use super::PackageIdSpec;
use crate::core::interning::InternedString;
use crate::core::{PackageId, SourceId};
use crate::util::ToSemver;
use url::Url;
Expand All @@ -288,63 +290,63 @@ mod tests {
ok(
"https://crates.io/foo#1.2.3",
PackageIdSpec {
name: "foo".to_string(),
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
ok(
"https://crates.io/foo#bar:1.2.3",
PackageIdSpec {
name: "bar".to_string(),
name: InternedString::new("bar"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo",
PackageIdSpec {
name: "foo".to_string(),
name: InternedString::new("foo"),
version: None,
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#1.2.3",
PackageIdSpec {
name: "foo".to_string(),
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#bar",
PackageIdSpec {
name: "bar".to_string(),
name: InternedString::new("bar"),
version: None,
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#bar:1.2.3",
PackageIdSpec {
name: "bar".to_string(),
name: InternedString::new("bar"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
},
);
ok(
"foo",
PackageIdSpec {
name: "foo".to_string(),
name: InternedString::new("foo"),
version: None,
url: None,
},
);
ok(
"foo:1.2.3",
PackageIdSpec {
name: "foo".to_string(),
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: None,
},
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl ProfileMaker {
let name_matches: Vec<String> = packages
.package_ids()
.filter_map(|pkg_id| {
if pkg_id.name().as_str() == spec.name() {
if pkg_id.name() == spec.name() {
Some(pkg_id.to_string())
} else {
None
Expand All @@ -292,7 +292,7 @@ impl ProfileMaker {
if name_matches.is_empty() {
let suggestion = packages
.package_ids()
.map(|p| (lev_distance(spec.name(), &p.name()), p.name()))
.map(|p| (lev_distance(&*spec.name(), &p.name()), p.name()))
.filter(|&(d, _)| d < 4)
.min_by_key(|p| p.0)
.map(|p| p.1);
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/conflict_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl ConflictCache {
/// all the `PackageId` entries are activated.
pub fn contains(&self, dep: &Dependency, con: &ConflictMap) -> bool {
if let Some(cst) = self.con_from_dep.get(dep) {
cst.contains(con.keys().cloned(), &con)
cst.contains(con.keys().cloned(), con)
} else {
false
}
Expand Down

0 comments on commit 0c891a0

Please sign in to comment.