Remove largely useless string interning support

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 All notable changes to MiniJinja are documented here.
 
+## 2.7.0
+
+- Removed string interning.  #675
+
 ## 2.6.0
 
 - Added `sum` filter.  #648

Makefile

@@ -15,7 +15,7 @@ doc:
 .PHONY: test-msrv
 test-msrv:
 	@$(MAKE) run-tests FEATURES=$(TEST_FEATURES)
-	@$(MAKE) run-tests FEATURES=$(TEST_FEATURES),preserve_order,key_interning,unicode
+	@$(MAKE) run-tests FEATURES=$(TEST_FEATURES),preserve_order,unicode
 	@echo "CARGO TEST ALL FEATURES"
 	@cd minijinja; cargo test --all-features
 

README.md

@@ -90,7 +90,7 @@ the engine is used so you can see how it's utilized:
   * **[Oranda](https://github.com/axodotdev/oranda)** uses it to [generate HTML landing pages](https://github.com/axodotdev/oranda/blob/fb97859c99ab81f644ab5b1449f725fc5c3e9721/src/site/templates.rs)
 
 * Code Generation:
-  * **[OpenTelemetry's Weaver](https://github.com/open-telemetry/weaver)** uses it to [generate documentation, code and other outputs](https://github.com/open-telemetry/weaver/blob/d49881445e09beb42e1a394bfa5f3068c660daf3/crates/weaver_forge/src/lib.rs#L482-L567) from the OTel specificiation.
+  * **[OpenTelemetry's Weaver](https://github.com/open-telemetry/weaver)** uses it to [generate documentation, code and other outputs](https://github.com/open-telemetry/weaver/blob/d49881445e09beb42e1a394bfa5f3068c660daf3/crates/weaver_forge/src/lib.rs#L482-L567) from the OTel specification.
 
 * Structure Generation:
   * **[Astral's Rye](https://rye.astral.sh/)** is using it to [generate project structures](https://github.com/astral-sh/rye/blob/c60682fb6bb5c9a0cc2669f263eeed99d2e5be71/rye/src/cli/init.rs)

minijinja/Cargo.toml

@@ -39,7 +39,6 @@ std_collections = []
 serde = []
 
 # Speedups
-key_interning = []
 speedups = ["v_htmlescape"]
 
 # Engine Features
@@ -54,6 +53,9 @@ fuel = []
 json = ["serde_json"]
 urlencode = ["percent-encoding"]
 
+# Deprecated features
+key_interning = []
+
 # Internal Features that should not be used
 internal_debug = []
 unstable_machinery = ["internal_debug"]

minijinja/src/lib.rs

@@ -187,12 +187,6 @@
 //!   limited.
 //! - `speedups`: enables all speedups, in particular it turns on the `v_htmlescape` dependency
 //!   for faster HTML escaping.
-//! - `key_interning`: if this feature is enabled the automatic string interning in
-//!   the value type is enabled.  This feature used to be turned on by default but
-//!   has negative performance effects in newer versions of MiniJinja since a lot of
-//!   the previous uses of key interning are no longer needed.  Enabling it however
-//!   cuts down on memory usage slightly in certain scenarios by interning all string
-//!   keys used in dynamic map values.
 //!
 //! Internals:
 //!

minijinja/src/macros.rs

@@ -28,11 +28,6 @@ pub mod __context {
     use crate::Environment;
     use std::rc::Rc;
 
-    #[inline(always)]
-    pub fn value_optimization() -> impl Drop {
-        crate::value::value_optimization()
-    }
-
     #[inline(always)]
     pub fn make() -> ValueMap {
         ValueMap::default()
@@ -137,7 +132,6 @@ macro_rules! context {
         $($key:ident $(=> $value:expr)?),*
         $(, .. $ctx:expr),* $(,)?
     ) => {{
-        let _guard = $crate::__context::value_optimization();
         let mut ctx = $crate::__context::make();
         $(
             $crate::__context_pair!(ctx, $key $(=> $value)?);
@@ -157,7 +151,6 @@ macro_rules! context {
     (
         $(.. $ctx:expr),* $(,)?
     ) => {{
-        let _guard = $crate::__context::value_optimization();
         let mut ctx = ::std::vec::Vec::new();
         $(
             ctx.push($crate::value::Value::from($ctx));

minijinja/src/template.rs

@@ -15,7 +15,7 @@ use crate::error::{attach_basic_debug_info, Error};
 use crate::output::{Output, WriteWrapper};
 use crate::syntax::SyntaxConfig;
 use crate::utils::AutoEscape;
-use crate::value::{self, Value};
+use crate::value::Value;
 use crate::vm::{prepare_blocks, Context, State, Vm};
 
 /// Callback for auto escape determination
@@ -372,9 +372,6 @@ impl<'source> CompiledTemplate<'source> {
         source: &'source str,
         config: &TemplateConfig,
     ) -> Result<CompiledTemplate<'source>, Error> {
-        // the parser/compiler combination can create constants in which case
-        // we can probably benefit from the value optimization a bit.
-        let _guard = value::value_optimization();
         let ast = ok!(parse(
             source,
             name,

minijinja/src/value/mod.rs

@@ -229,8 +229,6 @@ pub(crate) mod namespace_object;
 mod object;
 pub(crate) mod ops;
 mod serialize;
-#[cfg(feature = "key_interning")]
-mod string_interning;
 
 #[cfg(feature = "deserialization")]
 pub use self::deserialize::ViaDeserialize;
@@ -287,22 +285,6 @@ pub fn serializing_for_value() -> bool {
     INTERNAL_SERIALIZATION.with(|flag| flag.get())
 }
 
-/// Enables value optimizations.
-///
-/// If `key_interning` is enabled, this turns on that feature, otherwise
-/// it becomes a noop.
-#[inline(always)]
-pub(crate) fn value_optimization() -> impl Drop {
-    #[cfg(feature = "key_interning")]
-    {
-        crate::value::string_interning::use_string_cache()
-    }
-    #[cfg(not(feature = "key_interning"))]
-    {
-        OnDrop::new(|| {})
-    }
-}
-
 fn mark_internal_serialization() -> impl Drop {
     let old = INTERNAL_SERIALIZATION.with(|flag| {
         let old = flag.get();
@@ -685,36 +667,10 @@ impl Default for Value {
     }
 }
 
-/// Intern a string.
-///
-/// When the `key_interning` feature is in used, then MiniJinja will attempt to
-/// reuse strings in certain cases.  This function can be used to utilize the
-/// same functionality.  There is no guarantee that a string will be interned
-/// as there are heuristics involved for it.  Additionally the string interning
-/// will only work during the template engine execution (eg: within filters etc.).
-///
-/// The use of this function is generally recommended against and it might
-/// become deprecated in the future.
+#[doc(hidden)]
+#[deprecated = "This function no longer has an effect.  Use Arc::from directly."]
 pub fn intern(s: &str) -> Arc<str> {
-    #[cfg(feature = "key_interning")]
-    {
-        crate::value::string_interning::try_intern(s)
-    }
-    #[cfg(not(feature = "key_interning"))]
-    {
-        Arc::from(s.to_string())
-    }
-}
-
-/// Like [`intern`] but returns a [`Value`] instead of an `Arc<str>`.
-///
-/// This has the benefit that it will only perform interning if the string
-/// is not already interned.
-pub(crate) fn intern_into_value(s: &str) -> Value {
-    match SmallStr::try_new(s) {
-        Some(small_str) => Value(ValueRepr::SmallStr(small_str)),
-        None => Value::from(intern(s)),
-    }
+    Arc::from(s.to_string())
 }
 
 #[allow(clippy::len_without_is_empty)]
@@ -755,7 +711,6 @@ impl Value {
     /// serde deserialization.
     pub fn from_serialize<T: Serialize>(value: T) -> Value {
         let _serialization_guard = mark_internal_serialization();
-        let _optimization_guard = value_optimization();
         transform(value)
     }
 

minijinja/src/value/object.rs

@@ -5,7 +5,7 @@ use std::hash::Hash;
 use std::sync::Arc;
 
 use crate::error::{Error, ErrorKind};
-use crate::value::{intern, intern_into_value, mapped_enumerator, Value};
+use crate::value::{mapped_enumerator, Value};
 use crate::vm::State;
 
 /// A trait that represents a dynamic object.
@@ -297,7 +297,7 @@ macro_rules! impl_object_helpers {
                 }
                 Enumerator::Iter(iter) => Some(iter),
                 Enumerator::RevIter(iter) => Some(Box::new(iter)),
-                Enumerator::Str(s) => Some(Box::new(s.iter().copied().map(intern_into_value))),
+                Enumerator::Str(s) => Some(Box::new(s.iter().copied().map(Value::from))),
                 Enumerator::Values(v) => Some(Box::new(v.into_iter())),
             }
         }
@@ -736,9 +736,7 @@ macro_rules! impl_str_map_helper {
             }
 
             fn enumerate(self: &Arc<Self>) -> Enumerator {
-                self.$enumerator(|this| {
-                    Box::new(this.keys().map(|x| intern_into_value(x.as_ref())))
-                })
+                self.$enumerator(|this| Box::new(this.keys().map(|x| Value::from(x as &str))))
             }
 
             fn enumerator_len(self: &Arc<Self>) -> Option<usize> {
@@ -778,7 +776,7 @@ macro_rules! impl_str_map {
             fn from(val: $map_type<&'a str, V>) -> Self {
                 Value::from(
                     val.into_iter()
-                        .map(|(k, v)| (intern(k), v))
+                        .map(|(k, v)| (Arc::from(k), v))
                         .collect::<$map_type<Arc<str>, V>>(),
                 )
             }
@@ -791,15 +789,7 @@ macro_rules! impl_str_map {
             fn from(val: $map_type<Cow<'a, str>, V>) -> Self {
                 Value::from(
                     val.into_iter()
-                        .map(|(k, v)| {
-                            (
-                                match k {
-                                    Cow::Borrowed(s) => intern(s),
-                                    Cow::Owned(s) => Arc::<str>::from(s),
-                                },
-                                v,
-                            )
-                        })
+                        .map(|(k, v)| (Arc::from(k), v))
                         .collect::<$map_type<Arc<str>, V>>(),
                 )
             }

minijinja/src/vm/mod.rs

@@ -11,9 +11,7 @@ use crate::error::{Error, ErrorKind};
 use crate::output::{CaptureMode, Output};
 use crate::utils::{untrusted_size_hint, AutoEscape, UndefinedBehavior};
 use crate::value::namespace_object::Namespace;
-use crate::value::{
-    ops, value_map_with_capacity, value_optimization, Kwargs, ObjectRepr, Value, ValueMap,
-};
+use crate::value::{ops, value_map_with_capacity, Kwargs, ObjectRepr, Value, ValueMap};
 use crate::vm::context::{Frame, LoopState, Stack};
 use crate::vm::loop_object::Loop;
 use crate::vm::state::BlockStack;
@@ -91,7 +89,6 @@ impl<'env> Vm<'env> {
         out: &mut Output,
         auto_escape: AutoEscape,
     ) -> Result<(Option<Value>, State<'template, 'env>), Error> {
-        let _guard = value_optimization();
         let mut state = State::new(
             self.env,
             Context::new_with_frame(ok!(Frame::new_checked(root)), self.env.recursion_limit()),