Repository: mitsuhiko/minijinja (language: rust)

Issue:
Decide on i128/u128 in Value Type
With #142 the size of the `Value` type increased as i128/u128 are now inlined rather than in an `Arc`. This also increases the size of an `Instruction` from 32 bytes to 48 bytes on mac. For some reason the sizes appear to still be as expected on Linux (according to GHA at least).

The changes from #142 entirely results in noticeable performance differences for parse and compile (regression) but and render (improvements):

```
cmp_compile/minijinja   time:   [6.6548 µs 6.6674 µs 6.6817 µs]
                        change: [+1.2169% +1.5215% +1.8229%] (p = 0.00 < 0.05)
                        Performance has regressed.

cmp_render/minijinja    time:   [6.0559 µs 6.0681 µs 6.0793 µs]
                        change: [-3.8201% -3.5456% -3.2949%] (p = 0.00 < 0.05)
                        Performance has improved.

parse                   time:   [8.1028 µs 8.1185 µs 8.1328 µs]
                        change: [+1.1411% +1.3896% +1.6345%] (p = 0.00 < 0.05)
                        Performance has regressed.

compile                 time:   [12.986 µs 13.019 µs 13.054 µs]
                        change: [+1.3967% +1.6597% +1.9237%] (p = 0.00 < 0.05)
                        Performance has regressed.
```


Discussion / hints:
So looks like on x86_64 the thing still fits into the original sizes, on aarch64 it doesn't. That seems to be because the i128/u128 alignment is 16 on aarch64 vs 8 on x86_64.

Fix patch:
diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs
index cfb247de..47c362c8 100644
--- a/minijinja/src/filters.rs
+++ b/minijinja/src/filters.rs
@@ -492,7 +492,7 @@ mod builtins {
     pub fn abs(value: Value) -> Result<Value, Error> {
         match value.0 {
             ValueRepr::I64(x) => Ok(Value::from(x.abs())),
-            ValueRepr::I128(x) => Ok(Value::from(x.abs())),
+            ValueRepr::I128(x) => Ok(Value::from(x.0.abs())),
             ValueRepr::F64(x) => Ok(Value::from(x.abs())),
             _ => Err(Error::new(
                 ErrorKind::InvalidOperation,
diff --git a/minijinja/src/key/mod.rs b/minijinja/src/key/mod.rs
index 9f256194..a8a1ff56 100644
--- a/minijinja/src/key/mod.rs
+++ b/minijinja/src/key/mod.rs
@@ -82,11 +82,11 @@ impl<'a> Key<'a> {
             ValueRepr::U64(v) => TryFrom::try_from(v)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
-            ValueRepr::U128(v) => TryFrom::try_from(v)
+            ValueRepr::U128(v) => TryFrom::try_from(v.0)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
             ValueRepr::I64(v) => Ok(Key::I64(v)),
-            ValueRepr::I128(v) => TryFrom::try_from(v)
+            ValueRepr::I128(v) => TryFrom::try_from(v.0)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
             ValueRepr::F64(x) => {
diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs
index a2104279..79915c5a 100644
--- a/minijinja/src/value/argtypes.rs
+++ b/minijinja/src/value/argtypes.rs
@@ -5,7 +5,9 @@ use std::ops::{Deref, DerefMut};
 
 use crate::error::{Error, ErrorKind};
 use crate::key::{Key, StaticKey};
-use crate::value::{Arc, MapType, Object, StringType, Value, ValueKind, ValueRepr};
+use crate::value::{
+    Arc, MapType, MisalignedI128, MisalignedU128, Object, StringType, Value, ValueKind, ValueRepr,
+};
 use crate::vm::State;
 
 /// A utility trait that represents the return value of functions and filters.
@@ -291,17 +293,29 @@ macro_rules! value_from {
     };
 }
 
+impl From<i128> for Value {
+    #[inline(always)]
+    fn from(val: i128) -> Self {
+        ValueRepr::I128(MisalignedI128(val)).into()
+    }
+}
+
+impl From<u128> for Value {
+    #[inline(always)]
+    fn from(val: u128) -> Self {
+        ValueRepr::U128(MisalignedU128(val)).into()
+    }
+}
+
 value_from!(bool, Bool);
 value_from!(u8, U64);
 value_from!(u16, U64);
 value_from!(u32, U64);
 value_from!(u64, U64);
-value_from!(u128, U128);
 value_from!(i8, I64);
 value_from!(i16, I64);
 value_from!(i32, I64);
 value_from!(i64, I64);
-value_from!(i128, I128);
 value_from!(f32, F64);
 value_from!(f64, F64);
 value_from!(char, Char);
@@ -351,8 +365,8 @@ macro_rules! primitive_int_try_from {
             ValueRepr::U64(val) => val,
             // for the intention here see Key::from_borrowed_value
             ValueRepr::F64(val) if (val as i64 as f64 == val) => val as i64,
-            ValueRepr::I128(val) => val,
-            ValueRepr::U128(val) => val,
+            ValueRepr::I128(val) => val.0,
+            ValueRepr::U128(val) => val.0,
         });
     }
 }
diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs
index d0bbb76a..660c34f2 100644
--- a/minijinja/src/value/mod.rs
+++ b/minijinja/src/value/mod.rs
@@ -254,6 +254,18 @@ pub(crate) enum StringType {
     Safe,
 }
 
+/// An `i128` that intentionally uses packed layout to ensure
+/// that `ValueRepr` doesn't blow up in size.
+#[derive(Copy, Clone)]
+#[repr(packed)]
+pub(crate) struct MisalignedI128(pub i128);
+
+/// An `u128` that intentionally uses packed layout to ensure
+/// that `ValueRepr` doesn't blow up in size.
+#[derive(Copy, Clone)]
+#[repr(packed)]
+pub(crate) struct MisalignedU128(pub u128);
+
 #[derive(Clone)]
 pub(crate) enum ValueRepr {
     Undefined,
@@ -263,8 +275,8 @@ pub(crate) enum ValueRepr {
     F64(f64),
     Char(char),
     None,
-    U128(u128),
-    I128(i128),
+    U128(MisalignedU128),
+    I128(MisalignedI128),
     String(Arc<String>, StringType),
     Bytes(Arc<Vec<u8>>),
     Seq(Arc<Vec<Value>>),
@@ -282,8 +294,8 @@ impl fmt::Debug for ValueRepr {
             ValueRepr::F64(val) => fmt::Debug::fmt(val, f),
             ValueRepr::Char(val) => fmt::Debug::fmt(val, f),
             ValueRepr::None => write!(f, "None"),
-            ValueRepr::U128(val) => fmt::Debug::fmt(val, f),
-            ValueRepr::I128(val) => fmt::Debug::fmt(val, f),
+            ValueRepr::U128(val) => fmt::Debug::fmt(&{ val.0 }, f),
+            ValueRepr::I128(val) => fmt::Debug::fmt(&{ val.0 }, f),
             ValueRepr::String(val, _) => fmt::Debug::fmt(val, f),
             ValueRepr::Bytes(val) => fmt::Debug::fmt(val, f),
             ValueRepr::Seq(val) => fmt::Debug::fmt(val, f),
@@ -359,7 +371,7 @@ impl fmt::Display for Value {
             }
             ValueRepr::Char(val) => write!(f, "{}", val),
             ValueRepr::None => write!(f, "none"),
-            ValueRepr::I128(val) => write!(f, "{}", val),
+            ValueRepr::I128(val) => write!(f, "{}", { val.0 }),
             ValueRepr::String(val, _) => write!(f, "{}", val),
             ValueRepr::Bytes(val) => write!(f, "{}", String::from_utf8_lossy(val)),
             ValueRepr::Seq(values) => {
@@ -382,7 +394,7 @@ impl fmt::Display for Value {
                 }
                 write!(f, "}}")
             }
-            ValueRepr::U128(val) => write!(f, "{}", val),
+            ValueRepr::U128(val) => write!(f, "{}", { val.0 }),
             ValueRepr::Dynamic(x) => write!(f, "{}", x),
         }
     }
@@ -523,9 +535,9 @@ impl Value {
         match self.0 {
             ValueRepr::Bool(val) => val,
             ValueRepr::U64(x) => x != 0,
-            ValueRepr::U128(x) => x != 0,
+            ValueRepr::U128(x) => x.0 != 0,
             ValueRepr::I64(x) => x != 0,
-            ValueRepr::I128(x) => x != 0,
+            ValueRepr::I128(x) => x.0 != 0,
             ValueRepr::F64(x) => x != 0.0,
             ValueRepr::Char(x) => x != '\x00',
             ValueRepr::String(ref x, _) => !x.is_empty(),
@@ -820,11 +832,11 @@ impl Value {
             ValueRepr::U64(v) => TryFrom::try_from(v)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
-            ValueRepr::U128(v) => TryFrom::try_from(v)
+            ValueRepr::U128(v) => TryFrom::try_from(v.0)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
             ValueRepr::I64(v) => Ok(Key::I64(v)),
-            ValueRepr::I128(v) => TryFrom::try_from(v)
+            ValueRepr::I128(v) => TryFrom::try_from(v.0)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
             ValueRepr::Char(c) => Ok(Key::Char(c)),
@@ -898,8 +910,8 @@ impl Serialize for Value {
             ValueRepr::Char(c) => serializer.serialize_char(c),
             ValueRepr::None => serializer.serialize_unit(),
             ValueRepr::Undefined => serializer.serialize_unit(),
-            ValueRepr::U128(u) => serializer.serialize_u128(u),
-            ValueRepr::I128(i) => serializer.serialize_i128(i),
+            ValueRepr::U128(u) => serializer.serialize_u128(u.0),
+            ValueRepr::I128(i) => serializer.serialize_i128(i.0),
             ValueRepr::String(ref s, _) => serializer.serialize_str(s),
             ValueRepr::Bytes(ref b) => serializer.serialize_bytes(b),
             ValueRepr::Seq(ref elements) => elements.serialize(serializer),
diff --git a/minijinja/src/value/ops.rs b/minijinja/src/value/ops.rs
index 42d9f150..30599dc3 100644
--- a/minijinja/src/value/ops.rs
+++ b/minijinja/src/value/ops.rs
@@ -14,9 +14,9 @@ fn as_f64(value: &Value) -> Option<f64> {
     Some(match value.0 {
         ValueRepr::Bool(x) => x as i64 as f64,
         ValueRepr::U64(x) => x as f64,
-        ValueRepr::U128(x) => x as f64,
+        ValueRepr::U128(x) => x.0 as f64,
         ValueRepr::I64(x) => x as f64,
-        ValueRepr::I128(x) => x as f64,
+        ValueRepr::I128(x) => x.0 as f64,
         ValueRepr::F64(x) => x,
         _ => return None,
     })
@@ -27,13 +27,13 @@ pub fn coerce(a: &Value, b: &Value) -> Option<CoerceResult> {
         // equal mappings are trivial
         (ValueRepr::U64(a), ValueRepr::U64(b)) => Some(CoerceResult::I128(*a as i128, *b as i128)),
         (ValueRepr::U128(a), ValueRepr::U128(b)) => {
-            Some(CoerceResult::I128(*a as i128, *b as i128))
+            Some(CoerceResult::I128(a.0 as i128, b.0 as i128))
         }
         (ValueRepr::String(a, _), ValueRepr::String(b, _)) => {
             Some(CoerceResult::String(a.to_string(), b.to_string()))
         }
         (ValueRepr::I64(a), ValueRepr::I64(b)) => Some(CoerceResult::I128(*a as i128, *b as i128)),
-        (ValueRepr::I128(a), ValueRepr::I128(b)) => Some(CoerceResult::I128(*a, *b)),
+        (ValueRepr::I128(a), ValueRepr::I128(b)) => Some(CoerceResult::I128(a.0, b.0)),
         (ValueRepr::F64(a), ValueRepr::F64(b)) => Some(CoerceResult::F64(*a, *b)),
 
         // are floats involved?
diff --git a/minijinja/src/value/serialize.rs b/minijinja/src/value/serialize.rs
index 8e37a5b3..99bde51c 100644
--- a/minijinja/src/value/serialize.rs
+++ b/minijinja/src/value/serialize.rs
@@ -5,7 +5,8 @@ use serde::{ser, Serialize, Serializer};
 use crate::error::Error;
 use crate::key::{Key, KeySerializer, StaticKey};
 use crate::value::{
-    Arc, MapType, StringType, Value, ValueMap, ValueRepr, VALUE_HANDLES, VALUE_HANDLE_MARKER,
+    Arc, MapType, MisalignedI128, MisalignedU128, StringType, Value, ValueMap, ValueRepr,
+    VALUE_HANDLES, VALUE_HANDLE_MARKER,
 };
 
 pub struct ValueSerializer;
@@ -43,7 +44,7 @@ impl Serializer for ValueSerializer {
     }
 
     fn serialize_i128(self, v: i128) -> Result<Value, Error> {
-        Ok(ValueRepr::I128(v).into())
+        Ok(ValueRepr::I128(MisalignedI128(v)).into())
     }
 
     fn serialize_u8(self, v: u8) -> Result<Value, Error> {
@@ -63,7 +64,7 @@ impl Serializer for ValueSerializer {
     }
 
     fn serialize_u128(self, v: u128) -> Result<Value, Error> {
-        Ok(ValueRepr::U128(v).into())
+        Ok(ValueRepr::U128(MisalignedU128(v)).into())
     }
 
     fn serialize_f32(self, v: f32) -> Result<Value, Error> {


Test patch: