From c65967ced886e41cb1f63adda7f5f33cb020412f Mon Sep 17 00:00:00 2001 From: Yaroslav Bolyukin Date: Sun, 24 Apr 2022 20:30:52 +0000 Subject: [PATCH] refactor: better rebinding handling --- --- a/crates/jrsonnet-evaluator/src/ctx.rs +++ b/crates/jrsonnet-evaluator/src/ctx.rs @@ -4,34 +4,15 @@ use jrsonnet_interner::IStr; use crate::{ - cc_ptr_eq, error::Error::*, gc::GcHashMap, map::LayeredHashMap, LazyBinding, ObjValue, Pending, - Result, State, Thunk, Val, + cc_ptr_eq, error::Error::*, gc::GcHashMap, map::LayeredHashMap, ObjValue, Pending, Result, + Thunk, Val, }; -#[derive(Clone, Trace)] -pub struct ContextCreator(pub Context, pub Pending>); -impl ContextCreator { - pub fn create( - &self, - s: State, - this: Option, - super_obj: Option, - ) -> Result { - self.0.clone().extend_unbound( - s, - self.1.clone().unwrap(), - self.0.dollar().clone().or_else(|| this.clone()), - this, - super_obj, - ) - } -} - #[derive(Trace)] struct ContextInternals { dollar: Option, + sup: Option, this: Option, - super_obj: Option, bindings: LayeredHashMap, } impl Debug for ContextInternals { @@ -56,14 +37,14 @@ } pub fn super_obj(&self) -> &Option { - &self.0.super_obj + &self.0.sup } pub fn new() -> Self { Self(Cc::new(ContextInternals { dollar: None, this: None, - super_obj: None, + sup: None, bindings: LayeredHashMap::default(), })) } @@ -92,11 +73,6 @@ let mut new_bindings = GcHashMap::with_capacity(1); new_bindings.insert(name, Thunk::evaluated(value)); self.extend(new_bindings, None, None, None) - } - - #[must_use] - pub fn with_this_super(self, new_this: ObjValue, new_super_obj: Option) -> Self { - self.extend(GcHashMap::new(), None, Some(new_this), new_super_obj) } #[must_use] @@ -104,13 +80,13 @@ self, new_bindings: GcHashMap>, new_dollar: Option, + new_sup: Option, new_this: Option, - new_super_obj: Option, ) -> Self { let ctx = &self.0; let dollar = new_dollar.or_else(|| ctx.dollar.clone()); let this = new_this.or_else(|| ctx.this.clone()); - let super_obj = new_super_obj.or_else(|| ctx.super_obj.clone()); + let sup = new_sup.or_else(|| ctx.sup.clone()); let bindings = if new_bindings.is_empty() { ctx.bindings.clone() } else { @@ -118,32 +94,10 @@ }; Self(Cc::new(ContextInternals { dollar, + sup, this, - super_obj, bindings, })) - } - #[must_use] - pub fn extend_bound(self, new_bindings: GcHashMap>) -> Self { - let new_this = self.0.this.clone(); - let new_super_obj = self.0.super_obj.clone(); - self.extend(new_bindings, None, new_this, new_super_obj) - } - pub fn extend_unbound( - self, - s: State, - new_bindings: GcHashMap, - new_dollar: Option, - new_this: Option, - new_super_obj: Option, - ) -> Result { - let this = new_this.or_else(|| self.0.this.clone()); - let super_obj = new_super_obj.or_else(|| self.0.super_obj.clone()); - let mut new = GcHashMap::with_capacity(new_bindings.len()); - for (k, v) in new_bindings.0 { - new.insert(k, v.evaluate(s.clone(), this.clone(), super_obj.clone())?); - } - Ok(self.extend(new, new_dollar, this, super_obj)) } } --- a/crates/jrsonnet-evaluator/src/evaluate/destructure.rs +++ b/crates/jrsonnet-evaluator/src/evaluate/destructure.rs @@ -11,12 +11,13 @@ Context, Pending, State, Thunk, Val, }; +#[allow(clippy::too_many_lines)] fn destruct( d: &Destruct, parent: Thunk, new_bindings: &mut GcHashMap>, ) -> Result<()> { - Ok(match d { + match d { Destruct::Full(v) => { let old = new_bindings.insert(v.clone(), parent); if old.is_some() { @@ -157,8 +158,6 @@ } #[cfg(feature = "exp-destruct")] Destruct::Object { fields, rest } => { - use jrsonnet_parser::DestructRest; - use crate::{obj::ObjValue, throw_runtime}; #[derive(Trace)] @@ -223,7 +222,8 @@ } } } - }) + } + Ok(()) } pub fn evaluate_dest( --- a/crates/jrsonnet-evaluator/src/evaluate/mod.rs +++ b/crates/jrsonnet-evaluator/src/evaluate/mod.rs @@ -1,7 +1,9 @@ +use std::rc::Rc; + use gcmodule::{Cc, Trace}; use jrsonnet_interner::IStr; use jrsonnet_parser::{ - ArgsDesc, AssertStmt, BindSpec, CompSpec, Destruct, Expr, FieldMember, ForSpecData, IfSpecData, + ArgsDesc, AssertStmt, BindSpec, CompSpec, Expr, FieldMember, ForSpecData, IfSpecData, LiteralType, LocExpr, Member, ObjBody, ParamsDesc, }; use jrsonnet_types::ValType; @@ -14,147 +16,13 @@ stdlib::{std_slice, BUILTINS}, tb, throw, typed::Typed, - val::{ArrValue, Thunk, ThunkValue}, - Bindable, Context, ContextCreator, GcHashMap, LazyBinding, ObjValue, ObjValueBuilder, - ObjectAssertion, Pending, Result, State, Val, + val::{ArrValue, CachedBindable, Thunk, ThunkValue}, + Bindable, Context, GcHashMap, ObjValue, ObjValueBuilder, ObjectAssertion, Pending, Result, + State, Val, }; pub mod destructure; pub mod operator; - -#[allow(clippy::too_many_lines)] -pub fn evaluate_binding(b: BindSpec, cctx: ContextCreator) -> Result<(IStr, LazyBinding)> { - match b { - BindSpec::Field { - into: Destruct::Full(name), - value, - } => { - #[derive(Trace)] - struct BindableNamedThunk { - this: Option, - super_obj: Option, - - cctx: ContextCreator, - name: IStr, - value: LocExpr, - } - impl ThunkValue for BindableNamedThunk { - type Output = Val; - fn get(self: Box, s: State) -> Result { - evaluate_named( - s.clone(), - self.cctx.create(s, self.this, self.super_obj)?, - &self.value, - self.name, - ) - } - } - - #[derive(Trace)] - struct BindableNamed { - cctx: ContextCreator, - name: IStr, - value: LocExpr, - } - impl Bindable for BindableNamed { - fn bind( - &self, - _: State, - this: Option, - super_obj: Option, - ) -> Result> { - Ok(Thunk::new(tb!(BindableNamedThunk { - this, - super_obj, - cctx: self.cctx.clone(), - name: self.name.clone(), - value: self.value.clone(), - }))) - } - } - - Ok(( - name.clone(), - LazyBinding::Bindable(Cc::new(tb!(BindableNamed { - cctx, - name: name.clone(), - value: value.clone(), - }))), - )) - } - #[cfg(feature = "exp-destruct")] - BindSpec::Field { into: _, .. } => { - use crate::throw_runtime; - throw_runtime!("destructuring is not yet supported here") - } - BindSpec::Function { - name, - params, - value, - } => { - #[derive(Trace)] - struct BindableMethodThunk { - this: Option, - super_obj: Option, - - cctx: ContextCreator, - name: IStr, - params: ParamsDesc, - value: LocExpr, - } - impl ThunkValue for BindableMethodThunk { - type Output = Val; - fn get(self: Box, s: State) -> Result { - Ok(evaluate_method( - self.cctx.create(s, self.this, self.super_obj)?, - self.name, - self.params, - self.value, - )) - } - } - - #[derive(Trace)] - struct BindableMethod { - cctx: ContextCreator, - name: IStr, - params: ParamsDesc, - value: LocExpr, - } - impl Bindable for BindableMethod { - fn bind( - &self, - _: State, - this: Option, - super_obj: Option, - ) -> Result> { - Ok(Thunk::::new(tb!(BindableMethodThunk { - this, - super_obj, - - cctx: self.cctx.clone(), - name: self.name.clone(), - params: self.params.clone(), - value: self.value.clone(), - }))) - } - } - - let params = params.clone(); - - Ok(( - name.clone(), - LazyBinding::Bindable(Cc::new(tb!(BindableMethod { - cctx, - name: name.clone(), - params, - value, - }))), - )) - } - } -} - pub fn evaluate_method(ctx: Context, name: IStr, params: ParamsDesc, body: LocExpr) -> Val { Val::Func(FuncVal::Normal(Cc::new(FuncDesc { name, @@ -218,28 +86,65 @@ Ok(()) } +trait CloneableBindable: Bindable + Clone {} + +fn evaluate_object_locals( + fctx: Pending, + locals: Rc>, +) -> impl CloneableBindable { + #[derive(Trace, Clone)] + struct WithObjectLocals { + fctx: Pending, + locals: Rc>, + } + impl CloneableBindable for WithObjectLocals {} + impl Bindable for WithObjectLocals { + type Bound = Context; + + fn bind( + &self, + _s: State, + sup: Option, + this: Option, + ) -> Result { + let fctx = Context::new_future(); + let mut new_bindings = GcHashMap::new(); + for b in self.locals.iter() { + evaluate_dest(b, fctx.clone(), &mut new_bindings)?; + } + + let ctx = self.fctx.unwrap(); + let new_dollar = ctx.dollar().clone().or_else(|| this.clone()); + + let ctx = ctx + .extend(new_bindings, new_dollar, sup, this) + .into_future(fctx); + + Ok(ctx) + } + } + + WithObjectLocals { fctx, locals } +} + #[allow(clippy::too_many_lines)] pub fn evaluate_member_list_object(s: State, ctx: Context, members: &[Member]) -> Result { - let new_bindings = Pending::new(); - let future_this = Pending::new(); - let cctx = ContextCreator(ctx.clone(), new_bindings.clone()); - { - let mut bindings: GcHashMap = GcHashMap::with_capacity(members.len()); - for r in members + let mut builder = ObjValueBuilder::new(); + let locals = Rc::new( + members .iter() .filter_map(|m| match m { - Member::BindStmt(b) => Some(b.clone()), + Member::BindStmt(bind) => Some(bind.clone()), _ => None, }) - .map(|b| evaluate_binding(b.clone(), cctx.clone())) - { - let (n, b) = r?; - bindings.insert(n, b); - } - new_bindings.fill(bindings); - } + .collect::>(), + ); + + let fctx = Context::new_future(); + + // We have single context for all fields, so we can cache binds + let uctx = CachedBindable::new(evaluate_object_locals(fctx.clone(), locals)); - let mut builder = ObjValueBuilder::new(); for member in members.iter() { match member { Member::Field(FieldMember { @@ -250,21 +155,22 @@ value, }) => { #[derive(Trace)] - struct ObjMemberBinding { - cctx: ContextCreator, + struct ObjMemberBinding { + uctx: B, value: LocExpr, name: IStr, } - impl Bindable for ObjMemberBinding { + impl> Bindable for ObjMemberBinding { + type Bound = Thunk; fn bind( &self, s: State, + sup: Option, this: Option, - super_obj: Option, ) -> Result> { Ok(Thunk::evaluated(evaluate_named( s.clone(), - self.cctx.create(s, this, super_obj)?, + self.uctx.bind(s, sup, this)?, &self.value, self.name.clone(), )?)) @@ -286,9 +192,9 @@ .bindable( s.clone(), tb!(ObjMemberBinding { - cctx: cctx.clone(), + uctx: uctx.clone(), value: value.clone(), - name, + name: name.clone() }), )?; } @@ -299,21 +205,22 @@ .. }) => { #[derive(Trace)] - struct ObjMemberBinding { - cctx: ContextCreator, + struct ObjMemberBinding { + uctx: B, value: LocExpr, params: ParamsDesc, name: IStr, } - impl Bindable for ObjMemberBinding { + impl> Bindable for ObjMemberBinding { + type Bound = Thunk; fn bind( &self, s: State, + sup: Option, this: Option, - super_obj: Option, ) -> Result> { Ok(Thunk::evaluated(evaluate_method( - self.cctx.create(s, this, super_obj)?, + self.uctx.bind(s, sup, this)?, self.name.clone(), self.params.clone(), self.value.clone(), @@ -334,40 +241,42 @@ .bindable( s.clone(), tb!(ObjMemberBinding { - cctx: cctx.clone(), + uctx: uctx.clone(), value: value.clone(), params: params.clone(), - name, + name: name.clone() }), )?; } Member::BindStmt(_) => {} Member::AssertStmt(stmt) => { #[derive(Trace)] - struct ObjectAssert { - cctx: ContextCreator, + struct ObjectAssert { + uctx: B, assert: AssertStmt, } - impl ObjectAssertion for ObjectAssert { + impl> ObjectAssertion for ObjectAssert { fn run( &self, s: State, + sup: Option, this: Option, - super_obj: Option, ) -> Result<()> { - let ctx = self.cctx.create(s.clone(), this, super_obj)?; + let ctx = self.uctx.bind(s.clone(), sup, this)?; evaluate_assert(s, ctx, &self.assert) } } builder.assert(tb!(ObjectAssert { - cctx: cctx.clone(), + uctx: uctx.clone(), assert: stmt.clone(), })); } } } let this = builder.build(); - future_this.fill(this.clone()); + let _ctx = ctx + .extend(GcHashMap::new(), None, None, Some(this.clone())) + .into_future(fctx); Ok(this) } @@ -375,44 +284,45 @@ Ok(match object { ObjBody::MemberList(members) => evaluate_member_list_object(s, ctx, members)?, ObjBody::ObjComp(obj) => { - let future_this = Pending::new(); let mut builder = ObjValueBuilder::new(); - evaluate_comp(s.clone(), ctx, &obj.compspecs, &mut |ctx| { - let new_bindings = Pending::new(); - let cctx = ContextCreator(ctx.clone(), new_bindings.clone()); - let mut bindings: GcHashMap = - GcHashMap::with_capacity(obj.pre_locals.len() + obj.post_locals.len()); - for r in obj - .pre_locals + let locals = Rc::new( + obj.pre_locals .iter() .chain(obj.post_locals.iter()) - .map(|b| evaluate_binding(b.clone(), cctx.clone())) - { - let (n, b) = r?; - bindings.insert(n, b); - } - new_bindings.fill(bindings.clone()); - let ctx = ctx.extend_unbound(s.clone(), bindings, None, None, None)?; + .cloned() + .collect::>(), + ); + let mut ctxs = vec![]; + evaluate_comp(s.clone(), ctx, &obj.compspecs, &mut |ctx| { let key = evaluate(s.clone(), ctx.clone(), &obj.key)?; + let fctx = Context::new_future(); + ctxs.push((ctx, fctx.clone())); + let uctx = evaluate_object_locals(fctx, locals.clone()); match key { Val::Null => {} Val::Str(n) => { #[derive(Trace)] - struct ObjCompBinding { - ctx: Context, + struct ObjCompBinding { + uctx: B, value: LocExpr, } - impl Bindable for ObjCompBinding { + impl> Bindable for ObjCompBinding { + type Bound = Thunk; fn bind( &self, s: State, + sup: Option, this: Option, - _super_obj: Option, ) -> Result> { Ok(Thunk::evaluated(evaluate( - s, - self.ctx.clone().extend(GcHashMap::new(), None, this, None), + s.clone(), + self.uctx.bind(s, sup, this.clone())?.extend( + GcHashMap::new(), + None, + None, + this, + ), &self.value, )?)) } @@ -424,7 +334,7 @@ .bindable( s.clone(), tb!(ObjCompBinding { - ctx, + uctx, value: obj.value.clone(), }), )?; @@ -436,7 +346,11 @@ })?; let this = builder.build(); - future_this.fill(this.clone()); + for (ctx, fctx) in ctxs { + let _ctx = ctx + .extend(GcHashMap::new(), None, None, Some(this.clone())) + .into_future(fctx); + } this } }) @@ -596,7 +510,7 @@ for b in bindings { evaluate_dest(b, fctx.clone(), &mut new_bindings)?; } - let ctx = ctx.extend_bound(new_bindings).into_future(fctx); + let ctx = ctx.extend(new_bindings, None, None, None).into_future(fctx); evaluate(s, ctx, &returned.clone())? } Arr(items) => { --- a/crates/jrsonnet-evaluator/src/function/mod.rs +++ b/crates/jrsonnet-evaluator/src/function/mod.rs @@ -139,11 +139,12 @@ ) -> Result { match self { Self::Id => { + #[allow(clippy::unnecessary_wraps)] #[builtin] - fn builtin_id(v: Any) -> Result { + const fn builtin_id(v: Any) -> Result { Ok(v) } - static ID: &'static builtin_id = &builtin_id {}; + static ID: &builtin_id = &builtin_id {}; ID.call(s, call_ctx, loc, args) } @@ -159,10 +160,10 @@ self.evaluate(s, Context::default(), CallLocation::native(), args, true) } - pub fn is_identity(&self) -> bool { + pub const fn is_identity(&self) -> bool { matches!(self, Self::Id) } - pub fn identity() -> Self { + pub const fn identity() -> Self { Self::Id } } --- a/crates/jrsonnet-evaluator/src/function/parse.rs +++ b/crates/jrsonnet-evaluator/src/function/parse.rs @@ -111,7 +111,7 @@ Ok(body_ctx .extend(passed_args, None, None, None) - .extend_bound(defaults) + .extend(defaults, None, None, None) .into_future(fctx)) } else { let body_ctx = body_ctx.extend(passed_args, None, None, None); --- a/crates/jrsonnet-evaluator/src/gc.rs +++ b/crates/jrsonnet-evaluator/src/gc.rs @@ -10,7 +10,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; /// Replacement for box, which assumes that the underlying type is [`Trace`] -/// Used in places, where Cc should be used instead, but it can't, because CoerceUnsiced is not stable +/// Used in places, where `Cc` should be used instead, but it can't, because `CoerceUnsiced` is not stable #[derive(Debug, Clone)] pub struct TraceBox(pub Box); #[macro_export] --- a/crates/jrsonnet-evaluator/src/lib.rs +++ b/crates/jrsonnet-evaluator/src/lib.rs @@ -61,17 +61,13 @@ pub use val::{ManifestFormat, Thunk, Val}; pub trait Bindable: Trace + 'static { - fn bind( - &self, - s: State, - this: Option, - super_obj: Option, - ) -> Result>; + type Bound; + fn bind(&self, s: State, sup: Option, this: Option) -> Result; } #[derive(Clone, Trace)] pub enum LazyBinding { - Bindable(Cc>), + Bindable(Cc>>>), Bound(Thunk), } @@ -84,11 +80,11 @@ pub fn evaluate( &self, s: State, + sup: Option, this: Option, - super_obj: Option, ) -> Result> { match self { - Self::Bindable(v) => v.bind(s, this, super_obj), + Self::Bindable(v) => v.bind(s, sup, this), Self::Bound(v) => Ok(v.clone()), } } @@ -345,7 +341,7 @@ for (name, value) in globals.iter() { new_bindings.insert(name.clone(), Thunk::evaluated(value.clone())); } - Context::new().extend_bound(new_bindings) + Context::new().extend(new_bindings, None, None, None) } /// Executes code creating a new stack frame --- a/crates/jrsonnet-evaluator/src/obj.rs +++ b/crates/jrsonnet-evaluator/src/obj.rs @@ -106,7 +106,7 @@ } pub trait ObjectAssertion: Trace { - fn run(&self, s: State, this: Option, super_obj: Option) -> Result<()>; + fn run(&self, s: State, super_obj: Option, this: Option) -> Result<()>; } // Field => This @@ -124,10 +124,11 @@ #[derive(Trace)] #[force_tracking] pub struct ObjValueInternals { - super_obj: Option, + sup: Option, + this: Option, + assertions: Cc>>, assertions_ran: RefCell>, - this_obj: Option, this_entries: Cc>, value_cache: RefCell>, } @@ -153,7 +154,7 @@ pub struct ObjValue(pub(crate) Cc); impl Debug for ObjValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(super_obj) = self.0.super_obj.as_ref() { + if let Some(super_obj) = self.0.sup.as_ref() { if f.alternate() { write!(f, "{:#?}", super_obj)?; } else { @@ -171,15 +172,15 @@ impl ObjValue { pub fn new( - super_obj: Option, + sup: Option, this_entries: Cc>, assertions: Cc>>, ) -> Self { Self(Cc::new(ObjValueInternals { - super_obj, + sup, + this: None, assertions, assertions_ran: RefCell::new(GcHashSet::new()), - this_obj: None, this_entries, value_cache: RefCell::new(GcHashMap::new()), })) @@ -188,15 +189,15 @@ Self::new(None, Cc::new(GcHashMap::new()), Cc::new(Vec::new())) } #[must_use] - pub fn extend_from(&self, super_obj: Self) -> Self { - match &self.0.super_obj { + pub fn extend_from(&self, sup: Self) -> Self { + match &self.0.sup { None => Self::new( - Some(super_obj), + Some(sup), self.0.this_entries.clone(), self.0.assertions.clone(), ), Some(v) => Self::new( - Some(v.extend_from(super_obj)), + Some(v.extend_from(sup)), self.0.this_entries.clone(), self.0.assertions.clone(), ), @@ -212,12 +213,12 @@ } #[must_use] - pub fn with_this(&self, this_obj: Self) -> Self { + pub fn with_this(&self, this: Self) -> Self { Self(Cc::new(ObjValueInternals { - super_obj: self.0.super_obj.clone(), + sup: self.0.sup.clone(), assertions: self.0.assertions.clone(), assertions_ran: RefCell::new(GcHashSet::new()), - this_obj: Some(this_obj), + this: Some(this), this_entries: self.0.this_entries.clone(), value_cache: RefCell::new(GcHashMap::new()), })) @@ -234,7 +235,7 @@ if !self.0.this_entries.is_empty() { return false; } - self.0.super_obj.as_ref().map_or(true, Self::is_empty) + self.0.sup.as_ref().map_or(true, Self::is_empty) } /// Run callback for every field found in object @@ -243,7 +244,7 @@ depth: SuperDepth, handler: &mut impl FnMut(SuperDepth, &IStr, &ObjMember) -> bool, ) -> bool { - if let Some(s) = &self.0.super_obj { + if let Some(s) = &self.0.sup { if s.enum_fields(depth.deeper(), handler) { return true; } @@ -332,13 +333,13 @@ Some(match &m.visibility { Visibility::Normal => self .0 - .super_obj + .sup .as_ref() .and_then(|super_obj| super_obj.field_visibility(name)) .unwrap_or(Visibility::Normal), v => *v, }) - } else if let Some(super_obj) = &self.0.super_obj { + } else if let Some(super_obj) = &self.0.sup { super_obj.field_visibility(name) } else { None @@ -348,7 +349,7 @@ fn has_field_include_hidden(&self, name: IStr) -> bool { if self.0.this_entries.contains_key(&name) { true - } else if let Some(super_obj) = &self.0.super_obj { + } else if let Some(super_obj) = &self.0.sup { super_obj.has_field_include_hidden(name) } else { false @@ -369,13 +370,12 @@ pub fn get(&self, s: State, key: IStr) -> Result> { self.run_assertions(s.clone())?; - self.get_raw(s, key, self.0.this_obj.as_ref()) + self.get_raw(s, key, self.0.this.clone().unwrap_or_else(|| self.clone())) } // pub fn extend_with(self, key: ) - fn get_raw(&self, s: State, key: IStr, real_this: Option<&Self>) -> Result> { - let real_this = real_this.unwrap_or(self); + fn get_raw(&self, s: State, key: IStr, real_this: Self) -> Result> { let cache_key = (key.clone(), WeakObjValue(real_this.0.downgrade())); if let Some(v) = self.0.value_cache.borrow().get(&cache_key) { @@ -397,17 +397,17 @@ .insert(cache_key.clone(), CacheValue::Errored(e.clone())); e }; - let value = match (self.0.this_entries.get(&key), &self.0.super_obj) { + let value = match (self.0.this_entries.get(&key), &self.0.sup) { (Some(k), None) => Ok(Some( self.evaluate_this(s, k, real_this).map_err(fill_error)?, )), (Some(k), Some(super_obj)) => { let our = self - .evaluate_this(s.clone(), k, real_this) + .evaluate_this(s.clone(), k, real_this.clone()) .map_err(fill_error)?; if k.add { super_obj - .get_raw(s.clone(), key, Some(real_this)) + .get_raw(s.clone(), key, real_this) .map_err(fill_error)? .map_or(Ok(Some(our.clone())), |v| { Ok(Some(evaluate_add_op(s.clone(), &v, &our)?)) @@ -416,7 +416,7 @@ Ok(Some(our)) } } - (None, Some(super_obj)) => super_obj.get_raw(s, key, Some(real_this)), + (None, Some(super_obj)) => super_obj.get_raw(s, key, real_this), (None, None) => Ok(None), } .map_err(fill_error)?; @@ -429,9 +429,9 @@ ); Ok(value) } - fn evaluate_this(&self, s: State, v: &ObjMember, real_this: &Self) -> Result { + fn evaluate_this(&self, s: State, v: &ObjMember, real_this: Self) -> Result { v.invoke - .evaluate(s.clone(), Some(real_this.clone()), self.0.super_obj.clone())? + .evaluate(s.clone(), self.0.sup.clone(), Some(real_this))? .evaluate(s) } @@ -439,13 +439,13 @@ if self.0.assertions_ran.borrow_mut().insert(real_this.clone()) { for assertion in self.0.assertions.iter() { if let Err(e) = - assertion.run(s.clone(), Some(real_this.clone()), self.0.super_obj.clone()) + assertion.run(s.clone(), self.0.sup.clone(), Some(real_this.clone())) { self.0.assertions_ran.borrow_mut().remove(real_this); return Err(e); } } - if let Some(super_obj) = &self.0.super_obj { + if let Some(super_obj) = &self.0.sup { super_obj.run_assertions_raw(s, real_this)?; } } @@ -458,6 +458,9 @@ pub fn ptr_eq(a: &Self, b: &Self) -> bool { cc_ptr_eq(&a.0, &b.0) } + pub fn downgrade(self) -> WeakObjValue { + WeakObjValue(self.0.downgrade()) + } } impl PartialEq for ObjValue { @@ -475,7 +478,7 @@ #[allow(clippy::module_name_repetitions)] pub struct ObjValueBuilder { - super_obj: Option, + sup: Option, map: GcHashMap, assertions: Vec>, next_field_index: FieldIndex, @@ -486,7 +489,7 @@ } pub fn with_capacity(capacity: usize) -> Self { Self { - super_obj: None, + sup: None, map: GcHashMap::with_capacity(capacity), assertions: Vec::new(), next_field_index: FieldIndex::default(), @@ -497,7 +500,7 @@ self } pub fn with_super(&mut self, super_obj: ObjValue) -> &mut Self { - self.super_obj = Some(super_obj); + self.sup = Some(super_obj); self } @@ -512,7 +515,7 @@ } pub fn build(self) -> ObjValue { - ObjValue::new(self.super_obj, Cc::new(self.map), Cc::new(self.assertions)) + ObjValue::new(self.sup, Cc::new(self.map), Cc::new(self.assertions)) } } impl Default for ObjValueBuilder { @@ -583,7 +586,11 @@ pub fn value(self, s: State, value: Val) -> Result<()> { self.binding(s, LazyBinding::Bound(Thunk::evaluated(value))) } - pub fn bindable(self, s: State, bindable: TraceBox) -> Result<()> { + pub fn bindable( + self, + s: State, + bindable: TraceBox>>, + ) -> Result<()> { self.binding(s, LazyBinding::Bindable(Cc::new(bindable))) } pub fn binding(self, s: State, binding: LazyBinding) -> Result<()> { @@ -606,7 +613,7 @@ pub fn value(self, value: Val) { self.binding(LazyBinding::Bound(Thunk::evaluated(value))); } - pub fn bindable(self, bindable: TraceBox) { + pub fn bindable(self, bindable: TraceBox>>) { self.binding(LazyBinding::Bindable(Cc::new(bindable))); } pub fn binding(self, binding: LazyBinding) { --- a/crates/jrsonnet-evaluator/src/stdlib/mod.rs +++ b/crates/jrsonnet-evaluator/src/stdlib/mod.rs @@ -466,7 +466,7 @@ Ok(ArrValue::Eager(sort::sort( s.clone(), arr.evaluated(s)?, - keyF.unwrap_or(FuncVal::identity()), + keyF.unwrap_or_else(FuncVal::identity), )?)) } --- a/crates/jrsonnet-evaluator/src/stdlib/sort.rs +++ b/crates/jrsonnet-evaluator/src/stdlib/sort.rs @@ -68,7 +68,23 @@ if values.len() <= 1 { return Ok(values); } - if !key_getter.is_identity() { + if key_getter.is_identity() { + // Fast path, identity key getter + let mut values = (*values).clone(); + let sort_type = get_sort_type(&mut values, |k| k)?; + match sort_type { + SortKeyType::Number => values.sort_unstable_by_key(|v| match v { + Val::Num(n) => NonNaNf64(*n), + _ => unreachable!(), + }), + SortKeyType::String => values.sort_unstable_by_key(|v| match v { + Val::Str(s) => s.clone(), + _ => unreachable!(), + }), + SortKeyType::Unknown => unreachable!(), + }; + Ok(Cc::new(values)) + } else { // Slow path, user provided key getter let mut vk = Vec::with_capacity(values.len()); for value in values.iter() { @@ -90,21 +106,5 @@ SortKeyType::Unknown => unreachable!(), }; Ok(Cc::new(vk.into_iter().map(|v| v.0).collect())) - } else { - // Fast path, identity key getter - let mut values = (*values).clone(); - let sort_type = get_sort_type(&mut values, |k| k)?; - match sort_type { - SortKeyType::Number => values.sort_unstable_by_key(|v| match v { - Val::Num(n) => NonNaNf64(*n), - _ => unreachable!(), - }), - SortKeyType::String => values.sort_unstable_by_key(|v| match v { - Val::Str(s) => s.clone(), - _ => unreachable!(), - }), - SortKeyType::Unknown => unreachable!(), - }; - Ok(Cc::new(values)) } } --- a/crates/jrsonnet-evaluator/src/val.rs +++ b/crates/jrsonnet-evaluator/src/val.rs @@ -8,11 +8,11 @@ cc_ptr_eq, error::{Error::*, LocError}, function::FuncVal, - gc::TraceBox, + gc::{GcHashMap, TraceBox}, stdlib::manifest::{ manifest_json_ex, manifest_yaml_ex, ManifestJsonOptions, ManifestType, ManifestYamlOptions, }, - throw, ObjValue, Result, State, + throw, Bindable, ObjValue, Result, State, WeakObjValue, }; pub trait ThunkValue: Trace { @@ -71,6 +71,47 @@ } } +type CacheKey = (Option, Option); + +#[derive(Trace, Clone)] +pub struct CachedBindable +where + I: Bindable, +{ + cache: Cc>>, + value: I, +} +impl, T: Trace> CachedBindable { + pub fn new(value: I) -> Self { + Self { + cache: Cc::new(RefCell::new(GcHashMap::new())), + value, + } + } +} +impl, T: Clone + Trace> Bindable for CachedBindable { + type Bound = T; + fn bind(&self, s: State, sup: Option, this: Option) -> Result { + let cache_key = ( + sup.as_ref().map(|s| s.clone().downgrade()), + this.as_ref().map(|t| t.clone().downgrade()), + ); + { + if let Some(t) = self.cache.borrow().get(&cache_key) { + return Ok(t.clone()); + } + } + let bound = self.value.bind(s, sup, this)?; + + { + let mut cache = self.cache.borrow_mut(); + cache.insert(cache_key, bound.clone()); + } + + Ok(bound) + } +} + impl Debug for Thunk { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "Lazy") --- a/crates/jrsonnet-evaluator/tests/builtin.rs +++ b/crates/jrsonnet-evaluator/tests/builtin.rs @@ -6,7 +6,6 @@ use jrsonnet_evaluator::{ error::Result, function::{builtin, builtin::Builtin, CallLocation, FuncVal}, - gc::TraceBox, tb, typed::Typed, State, Val, --- a/crates/jrsonnet-evaluator/tests/common.rs +++ b/crates/jrsonnet-evaluator/tests/common.rs @@ -30,8 +30,18 @@ if !::jrsonnet_evaluator::val::equals($s.clone(), &$a.clone(), &$b.clone())? { ::jrsonnet_evaluator::throw_runtime!( "assertion failed: a != b\na={:#?}\nb={:#?}", - $a.to_json($s.clone(), 2)?, - $b.to_json($s.clone(), 2)?, + $a.to_json( + $s.clone(), + 2, + #[cfg(feature = "exp-preserve-order")] + false + )?, + $b.to_json( + $s.clone(), + 2, + #[cfg(feature = "exp-preserve-order")] + false + )?, ) } }}; --- a/crates/jrsonnet-evaluator/tests/golden.rs +++ b/crates/jrsonnet-evaluator/tests/golden.rs @@ -24,7 +24,12 @@ Ok(v) => v, Err(e) => return s.stringify_err(&e), }; - match v.to_json(s.clone(), 3) { + match v.to_json( + s.clone(), + 3, + #[cfg(feature = "exp-preserve-order")] + false, + ) { Ok(v) => v.to_string(), Err(e) => s.stringify_err(&e), } --- a/crates/jrsonnet-parser/src/lib.rs +++ b/crates/jrsonnet-parser/src/lib.rs @@ -165,7 +165,7 @@ / string_block() } / expected!("") pub rule field_name(s: &ParserSettings) -> expr::FieldName - = name:id() {expr::FieldName::Fixed(name.into())} + = name:id() {expr::FieldName::Fixed(name)} / name:string() {expr::FieldName::Fixed(name.into())} / "[" _ expr:expr(s) _ "]" {expr::FieldName::Dyn(expr)} pub rule visibility() -> expr::Visibility -- gitstuff