Skip to content

Commit

Permalink
Make sync mode the default and remove non-sync mode (#104)
Browse files Browse the repository at this point in the history
  • Loading branch information
mitsuhiko committed Sep 4, 2022
1 parent 0ac69a0 commit 25a3f0e
Show file tree
Hide file tree
Showing 15 changed files with 112 additions and 128 deletions.
9 changes: 5 additions & 4 deletions minijinja/Cargo.toml
Expand Up @@ -16,18 +16,19 @@ features = ["source", "json", "urlencode"]
rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "doc-header.html"]

[features]
default = ["builtins", "sync", "debug", "key_interning", "deserialization"]
sync = []
default = ["builtins", "debug", "key_interning", "deserialization"]
deserialization = []
key_interning = ["sync"]
key_interning = []
preserve_order = ["indexmap"]
debug = ["sync"]
debug = []
speedups = ["v_htmlescape"]
source = ["self_cell", "memo-map"]
builtins = []
json = ["serde_json"]
urlencode = ["percent-encoding"]

# this feature no longer has any effect
sync = []
# enables the Debug trait for some internal types
internal_debug = []
# provides access to the unstable machinery
Expand Down
11 changes: 6 additions & 5 deletions minijinja/src/environment.rs
@@ -1,5 +1,6 @@
use std::collections::BTreeMap;
use std::fmt;
use std::sync::Arc;

use serde::Serialize;

Expand All @@ -8,7 +9,7 @@ use crate::error::Error;
use crate::instructions::Instructions;
use crate::parser::{parse, parse_expr};
use crate::utils::{AutoEscape, BTreeMapKeysDebug, HtmlEscape};
use crate::value::{ArgType, FunctionArgs, RcType, Value};
use crate::value::{ArgType, FunctionArgs, Value};
use crate::vm::Vm;
use crate::{filters, functions, tests};

Expand Down Expand Up @@ -202,7 +203,7 @@ pub struct Environment<'source> {
filters: BTreeMap<&'source str, filters::BoxedFilter>,
tests: BTreeMap<&'source str, tests::BoxedTest>,
pub(crate) globals: BTreeMap<&'source str, Value>,
default_auto_escape: RcType<dyn Fn(&str) -> AutoEscape + Sync + Send>,
default_auto_escape: Arc<dyn Fn(&str) -> AutoEscape + Sync + Send>,
#[cfg(feature = "debug")]
debug: bool,
}
Expand Down Expand Up @@ -309,7 +310,7 @@ impl<'source> Environment<'source> {
filters: filters::get_builtin_filters(),
tests: tests::get_builtin_tests(),
globals: functions::get_globals(),
default_auto_escape: RcType::new(default_auto_escape),
default_auto_escape: Arc::new(default_auto_escape),
#[cfg(feature = "debug")]
debug: cfg!(debug_assertions),
}
Expand All @@ -325,7 +326,7 @@ impl<'source> Environment<'source> {
filters: Default::default(),
tests: Default::default(),
globals: Default::default(),
default_auto_escape: RcType::new(no_auto_escape),
default_auto_escape: Arc::new(no_auto_escape),
#[cfg(feature = "debug")]
debug: cfg!(debug_assertions),
}
Expand All @@ -349,7 +350,7 @@ impl<'source> Environment<'source> {
&mut self,
f: F,
) {
self.default_auto_escape = RcType::new(f);
self.default_auto_escape = Arc::new(f);
}

/// Enable or disable the debug mode.
Expand Down
7 changes: 4 additions & 3 deletions minijinja/src/filters.rs
Expand Up @@ -45,16 +45,17 @@
//! MiniJinja will perform the necessary conversions automatically via the
//! [`FunctionArgs`](crate::value::FunctionArgs) and [`Into`] traits.
use std::collections::BTreeMap;
use std::sync::Arc;

use crate::error::Error;
use crate::value::{ArgType, FunctionArgs, RcType, Value};
use crate::value::{ArgType, FunctionArgs, Value};
use crate::vm::State;
use crate::AutoEscape;

type FilterFunc = dyn Fn(&State, &Value, &[Value]) -> Result<Value, Error> + Sync + Send + 'static;

#[derive(Clone)]
pub(crate) struct BoxedFilter(RcType<FilterFunc>);
pub(crate) struct BoxedFilter(Arc<FilterFunc>);

/// A utility trait that represents filters.
pub trait Filter<V, Rv, Args>: Send + Sync + 'static {
Expand Down Expand Up @@ -93,7 +94,7 @@ impl BoxedFilter {
Rv: Into<Value>,
Args: for<'a> FunctionArgs<'a>,
{
BoxedFilter(RcType::new(
BoxedFilter(Arc::new(
move |state, value, args| -> Result<Value, Error> {
f.apply_to(
state,
Expand Down
15 changes: 8 additions & 7 deletions minijinja/src/key/mod.rs
Expand Up @@ -3,9 +3,10 @@ use std::convert::TryFrom;
use std::fmt;
use std::hash::{Hash, Hasher};
use std::num::TryFromIntError;
use std::sync::Arc;

use crate::error::{Error, ErrorKind};
use crate::value::{RcType, Value, ValueRepr};
use crate::value::{Value, ValueRepr};

pub use crate::key::serialize::KeySerializer;

Expand All @@ -19,7 +20,7 @@ pub enum Key<'a> {
Bool(bool),
I64(i64),
Char(char),
String(RcType<String>),
String(Arc<String>),
Str(&'a str),
}

Expand Down Expand Up @@ -53,7 +54,7 @@ impl<'a> Key<'a> {
}
#[cfg(not(feature = "key_interning"))]
{
Key::String(RcType::new(String::from(s)))
Key::String(Arc::new(String::from(s)))
}
}

Expand Down Expand Up @@ -197,7 +198,7 @@ pub mod key_interning {

enum CachedKey<'a> {
Ref(&'a str),
Stored(RcType<String>),
Stored(Arc<String>),
}

impl<'a> CachedKey<'a> {
Expand Down Expand Up @@ -237,22 +238,22 @@ pub mod key_interning {
})
}

pub(crate) fn try_intern(s: &str) -> RcType<String> {
pub(crate) fn try_intern(s: &str) -> Arc<String> {
let depth = STRING_KEY_CACHE_DEPTH.with(|depth| depth.load(Ordering::Relaxed));

// strings longer than 16 bytes are never interned or if we're at
// depth 0. (serialization code outside of internal serialization)
// not checking for depth can cause a memory leak.
if depth == 0 || s.len() > 16 {
return RcType::new(String::from(s));
return Arc::new(String::from(s));
}

STRING_KEY_CACHE.with(|cache| {
let mut set = cache.borrow_mut();
match set.get(&CachedKey::Ref(s)) {
Some(CachedKey::Stored(s)) => s.clone(),
None => {
let rv = RcType::new(String::from(s));
let rv = Arc::new(String::from(s));
set.insert(CachedKey::Stored(rv.clone()));
rv
}
Expand Down
5 changes: 0 additions & 5 deletions minijinja/src/lib.rs
Expand Up @@ -112,11 +112,6 @@
//!
//! - `builtins`: if this feature is removed the default filters, tests and
//! functions are not implemented.
//! - `sync`: this feature makes MiniJinja's type `Send` and `Sync`. If this feature
//! is disabled sending types across threads is often not possible. Thread bounds
//! of things like callbacks however are not changing which means code that uses
//! MiniJinja still needs to be threadsafe. This also disables some features that
//! require synchronization such as the `loop.changed` feature.
//! - `debug`: if this feature is removed some debug functionality of the engine is
//! removed as well. This mainly affects the quality of error reporting.
//! - `key_interning`: if this feature is removed the automatic string interning in
Expand Down
5 changes: 3 additions & 2 deletions minijinja/src/macros.rs
Expand Up @@ -5,8 +5,9 @@ use similar_asserts::assert_eq;
#[doc(hidden)]
pub mod __context {
use crate::key::Key;
use crate::value::{RcType, Value, ValueMap, ValueRepr};
use crate::value::{Value, ValueMap, ValueRepr};
use crate::Environment;
use std::sync::Arc;

#[inline(always)]
pub fn make() -> ValueMap {
Expand All @@ -20,7 +21,7 @@ pub mod __context {

#[inline(always)]
pub fn build(ctx: ValueMap) -> Value {
ValueRepr::Map(RcType::new(ctx)).into()
ValueRepr::Map(Arc::new(ctx)).into()
}

pub fn thread_local_env() -> Environment<'static> {
Expand Down
11 changes: 5 additions & 6 deletions minijinja/src/source.rs
Expand Up @@ -9,7 +9,6 @@ use self_cell::self_cell;

use crate::environment::CompiledTemplate;
use crate::error::{Error, ErrorKind};
use crate::value::RcType;

#[cfg(test)]
use similar_asserts::assert_eq;
Expand Down Expand Up @@ -39,11 +38,11 @@ pub struct Source {
#[derive(Clone)]
enum SourceBacking {
Dynamic {
templates: MemoMap<String, RcType<LoadedTemplate>>,
templates: MemoMap<String, Arc<LoadedTemplate>>,
loader: Arc<LoadFunc>,
},
Static {
templates: HashMap<String, RcType<LoadedTemplate>>,
templates: HashMap<String, Arc<LoadedTemplate>>,
},
}

Expand Down Expand Up @@ -162,10 +161,10 @@ impl Source {
SourceBacking::Dynamic {
ref mut templates, ..
} => {
templates.replace(name, RcType::new(tmpl));
templates.replace(name, Arc::new(tmpl));
}
SourceBacking::Static { ref mut templates } => {
templates.insert(name, RcType::new(tmpl));
templates.insert(name, Arc::new(tmpl));
}
}
Ok(())
Expand Down Expand Up @@ -262,7 +261,7 @@ impl Source {
LoadedTemplate::try_new(owner, |(name, source)| -> Result<_, Error> {
CompiledTemplate::from_name_and_source(name.as_str(), source)
})?;
Ok(RcType::new(tmpl))
Ok(Arc::new(tmpl))
})?
.borrow_dependent()),
SourceBacking::Static { templates } => templates
Expand Down
6 changes: 3 additions & 3 deletions minijinja/src/syntax.rs
Expand Up @@ -207,9 +207,9 @@
//! {% endfor %}
//! ```
//!
//! If the `sync` feature is not disabled, the `loop.changed` helper is also available
//! which can be used to detect when a value changes between the last iteration and the
//! current one. The method takes one or more arguments that are all compared.
//! A `loop.changed()` helper is also available which can be used to detect when
//! a value changes between the last iteration and the current one. The method
//! takes one or more arguments that are all compared.
//!
//! ```jinja
//! {% for entry in entries %}
Expand Down
23 changes: 11 additions & 12 deletions minijinja/src/tests.rs
Expand Up @@ -46,15 +46,16 @@
//! MiniJinja will perform the necessary conversions automatically via the
//! [`FunctionArgs`](crate::value::FunctionArgs) trait.
use std::collections::BTreeMap;
use std::sync::Arc;

use crate::error::Error;
use crate::value::{ArgType, FunctionArgs, RcType, Value};
use crate::value::{ArgType, FunctionArgs, Value};
use crate::vm::State;

type TestFunc = dyn Fn(&State, &Value, &[Value]) -> Result<bool, Error> + Sync + Send + 'static;

#[derive(Clone)]
pub(crate) struct BoxedTest(RcType<TestFunc>);
pub(crate) struct BoxedTest(Arc<TestFunc>);

/// A utility trait that represents filters.
pub trait Test<V, Args>: Send + Sync + 'static {
Expand Down Expand Up @@ -92,16 +93,14 @@ impl BoxedTest {
V: for<'a> ArgType<'a>,
Args: for<'a> FunctionArgs<'a>,
{
BoxedTest(RcType::new(
move |state, value, args| -> Result<bool, Error> {
let value = Some(value);
f.perform(
state,
ArgType::from_value(value)?,
FunctionArgs::from_values(args)?,
)
},
))
BoxedTest(Arc::new(move |state, value, args| -> Result<bool, Error> {
let value = Some(value);
f.perform(
state,
ArgType::from_value(value)?,
FunctionArgs::from_values(args)?,
)
}))
}

/// Applies the filter to a value and argument.
Expand Down
16 changes: 8 additions & 8 deletions minijinja/src/value/argtypes.rs
Expand Up @@ -4,7 +4,7 @@ use std::convert::TryFrom;

use crate::error::{Error, ErrorKind};
use crate::key::{Key, StaticKey};
use crate::value::{RcType, Value, ValueRepr};
use crate::value::{Arc, Value, ValueRepr};

/// Helper trait representing valid filter and test arguments.
///
Expand Down Expand Up @@ -76,21 +76,21 @@ impl From<ValueRepr> for Value {
impl<'a> From<&'a [u8]> for Value {
#[inline(always)]
fn from(val: &'a [u8]) -> Self {
ValueRepr::Bytes(RcType::new(val.into())).into()
ValueRepr::Bytes(Arc::new(val.into())).into()
}
}

impl<'a> From<&'a str> for Value {
#[inline(always)]
fn from(val: &'a str) -> Self {
ValueRepr::String(RcType::new(val.into())).into()
ValueRepr::String(Arc::new(val.into())).into()
}
}

impl From<String> for Value {
#[inline(always)]
fn from(val: String) -> Self {
ValueRepr::String(RcType::new(val)).into()
ValueRepr::String(Arc::new(val)).into()
}
}

Expand All @@ -114,14 +114,14 @@ impl From<()> for Value {
impl From<i128> for Value {
#[inline(always)]
fn from(val: i128) -> Self {
ValueRepr::I128(RcType::new(val)).into()
ValueRepr::I128(Arc::new(val)).into()
}
}

impl From<u128> for Value {
#[inline(always)]
fn from(val: u128) -> Self {
ValueRepr::U128(RcType::new(val)).into()
ValueRepr::U128(Arc::new(val)).into()
}
}

Expand All @@ -139,7 +139,7 @@ impl<'a> From<Key<'a>> for Value {

impl<K: Into<StaticKey>, V: Into<Value>> From<BTreeMap<K, V>> for Value {
fn from(val: BTreeMap<K, V>) -> Self {
ValueRepr::Map(RcType::new(
ValueRepr::Map(Arc::new(
val.into_iter().map(|(k, v)| (k.into(), v.into())).collect(),
))
.into()
Expand All @@ -148,7 +148,7 @@ impl<K: Into<StaticKey>, V: Into<Value>> From<BTreeMap<K, V>> for Value {

impl<T: Into<Value>> From<Vec<T>> for Value {
fn from(val: Vec<T>) -> Self {
ValueRepr::Seq(RcType::new(val.into_iter().map(|x| x.into()).collect())).into()
ValueRepr::Seq(Arc::new(val.into_iter().map(|x| x.into()).collect())).into()
}
}

Expand Down

0 comments on commit 25a3f0e

Please sign in to comment.