clarkmcc / cel-rust

Common Expression Language interpreter written in Rust
https://crates.io/crates/cel-interpreter
MIT License
375 stars 21 forks source link

Zero-allocation* redesign #76

Open clarkmcc opened 3 months ago

clarkmcc commented 3 months ago

This issue is meant to be a scratchpad for ideas and feedback on improving how types and values are handled in the interpreter. There have been several different feature requests recently that could be solved by the a fundamental shift in how CEL values are managed.

Today any value referenced in a CEL expression is owned by either the Program or the Context. The Program owns values that were created directly in the CEL expression, for example in the expression ("foo" == "bar") == false, there are three values owned by the Program, "foo", "bar", and false. Values that are owned by the Context are values that are dynamically provided at execution time to a Program as variables, like foo == bar, both foo and bar are values owned by the Context.

Questions:

kyle-mccarthy commented 2 months ago

While not zero-allocation, have you considered integrating with the protobuf ecosystem? There is prost-reflect which provides a DynamicMessage trait allowing field access via get_field. Program could then accept a DescriptorPool to help with field and type resolution.

Caellian commented 2 weeks ago

I like the idea of fundamentally changing Context so that it does not own any data, meaning that you do not need to clone your types to use them in CEL. Instead I'd like the Context to have references to that data.

This only works if you're not passing temporaries into Context, otherwise end-user is forced to keep (moving) them around for as long as the Context is used which is annoying and can be inefficient. I think simply using Cow for values would be simplest and most versatile, while also removing any extra copies/clones.

EDIT 1:

Also, if Value trait is added, and has add function, an additional add_to is required. In math, commutation is proven for numbers via induction and can't be assumed to hold true for every Value.

So the default implementation should be:

/// Called when other [`Value`] is added to this type.
#[inline]
fn add(&self, second: Value) -> ResolveResult {
    Err(ExecutionError::UnsupportedBinaryOperatorExternal(
        "add", std::any::type_name::<Self>(),
    ))
}

/// Called when this type is added to other [`Value`].
/// 
/// Override if addition is non-commutative.
#[inline]
fn add_to(&self, first: Value) -> ResolveResult {
    Self::add(self, first)
}