Auto merge of #5121 - Eh2406:string_interning, r=alexcrichton

String interning

This builds on the work from #5118. This interns the strings in the part of resolver that gets cloned a lot.

In a test on #4810 (comment)
Before we got to 1700000 ticks in ~(63 to 67) sec from #5118
After we got to 1700000 ticks in ~(42 to 45) sec

The interning code itself would be much better with a `leak` function that converts a `String` to a `&'static str`. Something like:
```rust
pub fn leek(s: String) -> &'static str {
    let ptr = s.as_ptr();
    let len = s.len();
    mem::forget(s);
    unsafe {
        let slice = slice::from_raw_parts(ptr, len);
        str::from_utf8(slice).unwrap()
    }
}
```
but "there is no unsafe in Cargo", and I am not the best at unsafe. So I just `to_string` and lived with the extra copy. Is there a better way to hand out references?

I assumed that `InternedString::new` world start appearing in profile result, and that we would want `PackageId`, and `Summary`, Et Al. to store the `InternedString`. That is why I put the interner in a shared folder. So far it is just used in the resolver. It may make sense for a lot more of the Strings to be interned, but with the extra copy... I have not explored it yet.

Author: bors

Cargo.toml

@@ -34,6 +34,7 @@ glob = "0.2"
 hex = "0.3"
 home = "0.3"
 ignore = "0.4"
+lazy_static = "1.0.0"
 jobserver = "0.1.9"
 lazycell = "0.6"
 libc = "0.2"

src/cargo/core/interning.rs

@@ -0,0 +1,68 @@
+use std::sync::RwLock;
+use std::collections::HashSet;
+use std::slice;
+use std::str;
+use std::mem;
+use std::cmp::Ordering;
+use std::ops::Deref;
+
+pub fn leek(s: String) -> &'static str {
+    let boxed = s.into_boxed_str();
+    let ptr = boxed.as_ptr();
+    let len = boxed.len();
+    mem::forget(boxed);
+    unsafe {
+        let slice = slice::from_raw_parts(ptr, len);
+        str::from_utf8_unchecked(slice)
+    }
+}
+
+lazy_static! {
+    static ref STRING_CASHE: RwLock<HashSet<&'static str>> =
+        RwLock::new(HashSet::new());
+}
+
+#[derive(Eq, PartialEq, Hash, Clone, Copy)]
+pub struct InternedString {
+    ptr: *const u8,
+    len: usize,
+}
+
+impl InternedString {
+    pub fn new(str: &str) -> InternedString {
+        let mut cache = STRING_CASHE.write().unwrap();
+        if let Some(&s) = cache.get(str) {
+            return InternedString { ptr: s.as_ptr(), len: s.len() };
+        }
+        let s = leek(str.to_string());
+        cache.insert(s);
+        InternedString { ptr: s.as_ptr(), len: s.len() }
+    }
+}
+
+impl Deref for InternedString {
+    type Target = str;
+
+    fn deref(&self) -> &'static str {
+        unsafe {
+            let slice = slice::from_raw_parts(self.ptr, self.len);
+            &str::from_utf8_unchecked(slice)
+        }
+    }
+}
+
+impl Ord for InternedString {
+    fn cmp(&self, other: &InternedString) -> Ordering {
+        let str: &str = &*self;
+        str.cmp(&*other)
+    }
+}
+
+impl PartialOrd for InternedString {
+    fn partial_cmp(&self, other: &InternedString) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+unsafe impl Send for InternedString {}
+unsafe impl Sync for InternedString {}

src/cargo/core/mod.rs

@@ -21,6 +21,7 @@ pub mod resolver;
 pub mod summary;
 pub mod shell;
 pub mod registry;
+mod interning;
 mod package_id_spec;
 mod workspace;
 mod features;

src/cargo/core/resolver/mod.rs

@@ -60,6 +60,7 @@ use url::Url;
 
 use core::{PackageId, Registry, SourceId, Summary, Dependency};
 use core::PackageIdSpec;
+use core::interning::InternedString;
 use util::config::Config;
 use util::Graph;
 use util::errors::{CargoResult, CargoError};
@@ -343,8 +344,8 @@ struct Context {
     //       switch to persistent hash maps if we can at some point or otherwise
     //       make these much cheaper to clone in general.
     activations: Activations,
-    resolve_features: HashMap<PackageId, HashSet<String>>,
-    links: HashMap<String, PackageId>,
+    resolve_features: HashMap<PackageId, HashSet<InternedString>>,
+    links: HashMap<InternedString, PackageId>,
 
     // These are two cheaply-cloneable lists (O(1) clone) which are effectively
     // hash maps but are built up as "construction lists". We'll iterate these
@@ -356,7 +357,7 @@ struct Context {
     warnings: RcList<String>,
 }
 
-type Activations = HashMap<String, HashMap<SourceId, Rc<Vec<Summary>>>>;
+type Activations = HashMap<InternedString, HashMap<SourceId, Rc<Vec<Summary>>>>;
 
 /// Builds the list of all packages required to build the first argument.
 pub fn resolve(summaries: &[(Summary, Method)],
@@ -382,7 +383,7 @@ pub fn resolve(summaries: &[(Summary, Method)],
         metadata: BTreeMap::new(),
         replacements: cx.resolve_replacements(),
         features: cx.resolve_features.iter().map(|(k, v)| {
-            (k.clone(), v.clone())
+            (k.clone(), v.iter().map(|x| x.to_string()).collect())
         }).collect(),
         unused_patches: Vec::new(),
     };
@@ -717,7 +718,7 @@ impl RemainingCandidates {
     fn next(
         &mut self,
         prev_active: &[Summary],
-        links: &HashMap<String, PackageId>,
+        links: &HashMap<InternedString, PackageId>,
     ) -> Result<(Candidate, bool), HashMap<PackageId, ConflictReason>> {
         // Filter the set of candidates based on the previously activated
         // versions for this dependency. We can actually use a version if it
@@ -734,7 +735,7 @@ impl RemainingCandidates {
         use std::mem::replace;
         for (_, b) in self.remaining.by_ref() {
             if let Some(link) = b.summary.links() {
-                if let Some(a) = links.get(link) {
+                if let Some(a) = links.get(&InternedString::new(link)) {
                     if a != b.summary.package_id() {
                         self.conflicting_prev_active
                             .entry(a.clone())
@@ -1304,14 +1305,14 @@ impl Context {
                       method: &Method) -> CargoResult<bool> {
         let id = summary.package_id();
         let prev = self.activations
-                       .entry(id.name().to_string())
+                       .entry(InternedString::new(id.name()))
                        .or_insert_with(HashMap::new)
                        .entry(id.source_id().clone())
                        .or_insert_with(||Rc::new(Vec::new()));
         if !prev.iter().any(|c| c == summary) {
             self.resolve_graph.push(GraphNode::Add(id.clone()));
             if let Some(link) = summary.links() {
-                ensure!(self.links.insert(link.to_owned(), id.clone()).is_none(),
+                ensure!(self.links.insert(InternedString::new(link), id.clone()).is_none(),
                 "Attempting to resolve a with more then one crate with the links={}. \n\
                  This will not build as is. Consider rebuilding the .lock file.", link);
             }
@@ -1332,8 +1333,8 @@ impl Context {
         let has_default_feature = summary.features().contains_key("default");
         Ok(match self.resolve_features.get(id) {
             Some(prev) => {
-                features.iter().all(|f| prev.contains(f)) &&
-                    (!use_default || prev.contains("default") ||
+                features.iter().all(|f| prev.contains(&InternedString::new(f))) &&
+                    (!use_default || prev.contains(&InternedString::new("default")) ||
                      !has_default_feature)
             }
             None => features.is_empty() && (!use_default || !has_default_feature)
@@ -1367,14 +1368,14 @@ impl Context {
     }
 
     fn prev_active(&self, dep: &Dependency) -> &[Summary] {
-        self.activations.get(dep.name())
+        self.activations.get(&InternedString::new(dep.name()))
             .and_then(|v| v.get(dep.source_id()))
             .map(|v| &v[..])
             .unwrap_or(&[])
     }
 
     fn is_active(&self, id: &PackageId) -> bool {
-        self.activations.get(id.name())
+        self.activations.get(&InternedString::new(id.name()))
             .and_then(|v| v.get(id.source_id()))
             .map(|v| v.iter().any(|s| s.package_id() == id))
             .unwrap_or(false)
@@ -1448,9 +1449,7 @@ impl Context {
             let set = self.resolve_features.entry(pkgid.clone())
                               .or_insert_with(HashSet::new);
             for feature in reqs.used {
-                if !set.contains(feature) {
-                    set.insert(feature.to_string());
-                }
+                set.insert(InternedString::new(feature));
             }
         }
 

src/cargo/lib.rs

@@ -27,6 +27,7 @@ extern crate home;
 extern crate ignore;
 extern crate jobserver;
 extern crate lazycell;
+#[macro_use] extern crate lazy_static;
 extern crate libc;
 extern crate libgit2_sys;
 extern crate num_cpus;