VM: Speed up Rust native function calls by using FnArgsVec (smallvec)

[schungx]

Use FnArgsVec to avoid allocations

This PR reduces Vec allocations during Rust native function calls by using FnArgsVec (which is a typedef to smallvec) instead to hold arguments. This generates a 50% speed-up for function calls.

[ImTheSquid]

I think we should maybe change this to be a grain subfeature grain-unchecked because malicious bytecode could panic the system here. We can do it at compile time to get rid of any burden of conditionals. See below

[ImTheSquid]

This crashes:

    /// PoC: a map template that is not a map. The verifier proves the operand
    /// is *there*, never that it is a `Map`, and the constant pool comes off
    /// the artifact.
    #[test]
    #[cfg(not(feature = "no_object"))]
    fn poc_make_map_template_type() {
        let program = one(
            &[Op::Const(0), Op::MakeMap(0), Op::Return],
            vec![Dynamic::from(42 as INT)],
        );
        assert!(program.verify().is_ok(), "the verifier accepts this chunk");
        let err = *call(&program, None).unwrap_err();
        println!("MAKE_MAP result: {err:?}");
    }

    /// PoC: a size check that claims to be element 1 of a literal that was
    /// never started. `State` tracks operands, iterators and handlers — there
    /// is no fourth counter for the size stack.
    #[test]
    #[cfg(not(feature = "unchecked"))]
    #[cfg(not(all(feature = "no_index", feature = "no_object")))]
    fn poc_check_size_without_a_literal() {
        let program = one(
            &[
                Op::Const(0),
                Op::CheckSize {
                    index: 1,
                    map: false,
                },
                Op::Return,
            ],
            vec![Dynamic::from(42 as INT)],
        );
        assert!(program.verify().is_ok(), "the verifier accepts this chunk");

        // A host that sets no limit never reads the running total, so the
        // reachable case is the one that does.
        let mut engine = Engine::new();
        engine.set_max_array_size(100);
        let mut options = CallFnOptions::new().eval_ast(false);
        options.this_ptr = None;
        let result: Result<Dynamic, _> =
            Vm::new(&engine).call_fn_with_options(options, &mut Scope::new(), &program, "f", ());
        println!("CHECK_SIZE result: {:?}", result.map(|_| ()));
    }

    /// PoC: a chain index step naming an operand slot the chain does not have.
    /// `check_chain_indices` skips `Step::Index` on the grounds that its
    /// operands are stack offsets, and nothing else bounds them.
    #[test]
    #[cfg(not(feature = "no_index"))]
    fn poc_chain_index_operand_out_of_range() {
        use crate::grain::bytecode::Root;

        let chain = Chain {
            root: Root::Temporary,
            steps: vec![Step::Index {
                operand: 500,
                pos: rhai::Position::NONE,
                bracket: rhai::Position::NONE,
            }],
            tail: Tail::Read,
            operands: 1,
        };
        let program = program_with_chains(
            &[&[Op::Const(0), Op::Const(1), Op::Chain(0), Op::Return]],
            vec![
                Dynamic::from(0 as INT),
                Dynamic::from_array(vec![Dynamic::from(7 as INT)]),
            ],
            vec![chain],
        );
        assert!(program.verify().is_ok(), "the verifier accepts this chunk");
        let err = *call(&program, None).unwrap_err();
        println!("CHAIN result: {err:?}");
    }
}

For the first one restoring the check should be fine, it's per-map literal. For the second and third you can move the check into the verifier. I take back needing a new feature for this, but we need to really prove correctness of the verifier now.

[schungx]

I think we should maybe change this to be a grain subfeature grain-unchecked because malicious bytecode could panic the system here. We can do it at compile time to get rid of any burden of conditionals.

I think we can do this with the unchecked feature. In that case we can leave all the _unchecked versions out and simply but gates on the version that return Result. Under unchecked, that simply returns Ok, which the compiler will optimize away the error check (I hope).

On the other hand, Rhai already has a whole bunch of places where panics may happen. The premise of Rhai has never been "no panics", but that it will not intentionally panic -- therefore any panic is a bug.

Here, I think the premise is the same: the bytecodes are verified to the best of effort, so panics should not happen. If it happens, then it is a bug in the VM that needs to be fixed.

[ImTheSquid]

Ok then let's at least fix the bugs I posted and then we can merge this, the speed gains are really good. We should probably do some more analysis on the verifier to harden it in the future.

[schungx]

I have reversed the error-handling changes. Now it is only the Vec allocations. We can merge this first.

I agree probably we would need some more scrutiny for the error-handling stuff. I'll open a new PR with an alternative way of doing it.

[ImTheSquid]

Let's put a pin in the verifier, I'd love to keep those speed gains but they need to be tested well.

[schungx]

Let's put a pin in the verifier, I'd love to keep those speed gains but they need to be tested well.

Yup. I think I'd do it in a different way that would keep all the error handling intact, but remove them in unchecked builds.

[schungx]

@ImTheSquid I think this one can be merged first. It does not involve introducing any potential panics.

[ImTheSquid]

Agreed