Auto merge of #14211 - Eh2406:default-without-lock, r=epage

dont make new constant InternedString in hot path

### What does this PR try to resolve?

`InternedString::new` always takes a `Mutex` lock. This is not a big deal because `new` is usually only called as external io is done (i.e not in a hot loop) and cargo is single threaded so the lock is not contested. However, dealing with the "default" feature happens deep within the resolver (i.e in a hot loop). So we may as well remove this overhead. I can get runtime benchmarks if that is desired.

For context: My pubgrub work would very much like to run cargoes resolver simultaneously in multiple threads. This `Mutex` is a major obstacle for that effort.

### How should we test and review this PR?

Internal re-factor and test still pass.

Also confirmed the semantics of `static` in https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Const.20evaluation.20and.20equality

### Additional information

There are other constant InternedString's in `src/cargo/core/profiles.rs`. I don't think they are in a hot enough path to justify more `static`s. But I will change if requested.

Author: bors

src/cargo/core/resolver/dep_cache.rs

@@ -21,7 +21,7 @@ use crate::core::{
 };
 use crate::sources::source::QueryKind;
 use crate::util::errors::CargoResult;
-use crate::util::interning::InternedString;
+use crate::util::interning::{InternedString, INTERNED_DEFAULT};
 
 use anyhow::Context as _;
 use std::collections::{BTreeSet, HashMap, HashSet};
@@ -348,7 +348,7 @@ fn build_requirements<'a, 'b: 'a>(
 
     let handle_default = |uses_default_features, reqs: &mut Requirements<'_>| {
         if uses_default_features && s.features().contains_key("default") {
-            if let Err(e) = reqs.require_feature(InternedString::new("default")) {
+            if let Err(e) = reqs.require_feature(INTERNED_DEFAULT) {
                 return Err(e.into_activate_error(parent, s));
             }
         }

src/cargo/core/resolver/features.rs

@@ -43,7 +43,7 @@ use crate::core::dependency::{ArtifactTarget, DepKind, Dependency};
 use crate::core::resolver::types::FeaturesSet;
 use crate::core::resolver::{Resolve, ResolveBehavior};
 use crate::core::{FeatureValue, PackageId, PackageIdSpec, PackageSet, Workspace};
-use crate::util::interning::InternedString;
+use crate::util::interning::{InternedString, INTERNED_DEFAULT};
 use crate::util::CargoResult;
 use anyhow::{bail, Context};
 use itertools::Itertools;
@@ -745,9 +745,8 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
             .iter()
             .map(|f| FeatureValue::new(*f))
             .collect();
-        let default = InternedString::new("default");
-        if dep.uses_default_features() && feature_map.contains_key(&default) {
-            result.push(FeatureValue::Feature(default));
+        if dep.uses_default_features() && feature_map.contains_key(&INTERNED_DEFAULT) {
+            result.push(FeatureValue::Feature(INTERNED_DEFAULT));
         }
         result
     }
@@ -762,9 +761,8 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
         let feature_map = summary.features();
 
         let mut result: Vec<FeatureValue> = cli_features.features.iter().cloned().collect();
-        let default = InternedString::new("default");
-        if cli_features.uses_default_features && feature_map.contains_key(&default) {
-            result.push(FeatureValue::Feature(default));
+        if cli_features.uses_default_features && feature_map.contains_key(&INTERNED_DEFAULT) {
+            result.push(FeatureValue::Feature(INTERNED_DEFAULT));
         }
 
         if cli_features.all_features {

src/cargo/ops/tree/graph.rs

@@ -6,7 +6,7 @@ use crate::core::dependency::DepKind;
 use crate::core::resolver::features::{CliFeatures, FeaturesFor, ResolvedFeatures};
 use crate::core::resolver::Resolve;
 use crate::core::{FeatureMap, FeatureValue, Package, PackageId, PackageIdSpec, Workspace};
-use crate::util::interning::InternedString;
+use crate::util::interning::{InternedString, INTERNED_DEFAULT};
 use crate::util::CargoResult;
 use std::collections::{HashMap, HashSet};
 
@@ -415,7 +415,7 @@ fn add_pkg(
                 if dep.uses_default_features() {
                     add_feature(
                         graph,
-                        InternedString::new("default"),
+                        INTERNED_DEFAULT,
                         Some(from_index),
                         dep_index,
                         EdgeKind::Dep(dep.kind()),
@@ -505,7 +505,7 @@ fn add_cli_features(
     }
 
     if cli_features.uses_default_features {
-        to_add.insert(FeatureValue::Feature(InternedString::new("default")));
+        to_add.insert(FeatureValue::Feature(INTERNED_DEFAULT));
     }
     to_add.extend(cli_features.features.iter().cloned());
 

src/cargo/util/interning.rs

@@ -13,7 +13,19 @@ use std::str;
 use std::sync::Mutex;
 use std::sync::OnceLock;
 
-static STRING_CACHE: OnceLock<Mutex<HashSet<&'static str>>> = OnceLock::new();
+pub static INTERNED_DEFAULT: InternedString = InternedString { inner: "default" };
+
+fn interned_storage() -> std::sync::MutexGuard<'static, HashSet<&'static str>> {
+    static STRING_CACHE: OnceLock<Mutex<HashSet<&'static str>>> = OnceLock::new();
+    STRING_CACHE
+        .get_or_init(|| {
+            let mut out: HashSet<&'static str> = Default::default();
+            out.insert(INTERNED_DEFAULT.as_str());
+            Mutex::new(out)
+        })
+        .lock()
+        .unwrap()
+}
 
 #[derive(Clone, Copy)]
 pub struct InternedString {
@@ -60,8 +72,8 @@ impl Eq for InternedString {}
 
 impl InternedString {
     pub fn new(str: &str) -> InternedString {
-        let mut cache = STRING_CACHE.get_or_init(Default::default).lock().unwrap();
-        let s = cache.get(str).cloned().unwrap_or_else(|| {
+        let mut cache = interned_storage();
+        let s = cache.get(str).copied().unwrap_or_else(|| {
             let s = str.to_string().leak();
             cache.insert(s);
             s