Make Grain function loading lazy, tighten up routines

CHANGELOG.md

@@ -18,6 +18,7 @@ Bug fixes
 * Assigning a constant into a container variable (array, map etc.) now works properly without making the value read-only (thanks [`@cuervolu`](https://github.com/cuervolu) [`#1187`](https://github.com/rhaiscript/rhai/pull/1187)).
 * Under `sync`, a method call on a shared variable with a closure argument that captures the same variable now raises `ErrorDataRace`, as it does without `sync`, instead of panicking while resolving the function. `Dynamic::is_locked` now returns `true` under `sync` when the value cannot be read, which includes being locked for write by the current thread (thanks [`@yuvalrakavy`](https://github.com/yuvalrakavy) [`#1190`](https://github.com/rhaiscript/rhai/pull/1190)).
 * A panic inside a native Rust function that takes its first argument by value, called with a variable first argument, now unwinds normally and can be caught by `catch_unwind`. Previously the copied first argument was restored only on the normal-return path, so the unwind tripped an internal assertion and the process aborted (thanks [`@yuvalrakavy`](https://github.com/yuvalrakavy) [`#1189`](https://github.com/rhaiscript/rhai/pull/1189)).
+* `AST::clone_functions_only_filtered` now applies its filter, as does `AST::merge_filtered` when the first `AST` has no functions. Previously every function was kept (thanks [`@ImTheSquid`](https://github.com/ImTheSquid) [`#1192`](https://github.com/rhaiscript/rhai/pull/1192)).
 
 Rhai Grain is now at feature parity with the AST interpreter
 ------------------------------------------------------------
@@ -49,6 +50,7 @@ Lazy loading of plugin module functions
 * Lazy-load plugin's are supported by both the `AST` interpreter and Rhai Grain.
 * Use the new `Module::from_manifest` (the lazy `exported_module!`) or `Module::combine_manifest` (the lazy `combine_with_exported_module!`) API to create a lazy-loaded module or add the manifest of lazy-loaded functions to a module.
 * `Module::register_lazy_functions` registers all lazy-loaded functions up-front instead, for the same speed as before.
+* A `Module` created by `Module::eval_grain_as_new` now looks up the functions of the Rhai Grain program only when called. The new `Vm::eval_with_lazy_callbacks` does the same for functions called back from native Rust functions, while `Vm::eval_with_callbacks` still registers every function up-front ([`#1192`](https://github.com/rhaiscript/rhai/pull/1192)).
 
 Feature flag to replace `smartstring` with `compact_str`
 --------------------------------------------------------

src/eval/expr.rs

@@ -109,16 +109,11 @@ impl Engine {
 
         // Finally, see if it is a script function name and return a function pointer
         #[cfg(not(feature = "no_function"))]
-        if let Some(func) = global
+        if let Some(fn_def) = global
             .lib
             .iter()
-            .flat_map(|m| m.iter_fn())
-            .filter(|(f, _)| f.is_script())
-            .filter(|(_, meta)| meta.name == var_name.as_str())
-            .map(|(f, _)| f)
-            .next()
+            .find_map(|m| m.get_script_fn_named(var_name, None))
         {
-            let fn_def = func.get_script_fn_def().unwrap();
             let val: Dynamic = crate::FnPtr {
                 name: var_name.clone(),
                 curry: <_>::default(),

src/grain/bytecode/positions.rs

@@ -15,12 +15,15 @@ use std::prelude::v1::*;
 /// instructions that had one. What it buys is that they can now be removed.
 /// `tests/format.rs` measures how much that removes.
 ///
-/// In memory this is dense, because the lookup is not always cold. A loop
-/// back-edge passes a position to `track_operation` on every iteration, and the
-/// built-in operator path builds a `NativeCallContext` around one — both on the
-/// hot path, both needing it only if something goes wrong. Indexing an array is
-/// what makes that free. The compact delta form in [`pos`](crate::grain::pos)
-/// is the wire form, expanded once at load.
+/// In memory this holds only the instructions that have a position, in
+/// address order. The lookup is not always cold — a loop back-edge passes a
+/// position to `track_operation` on every iteration, and the built-in operator
+/// path builds a `NativeCallContext` around one — so it is a binary search,
+/// a dozen comparisons for a large script. An entry per code byte would make
+/// it an index, at four bytes of heap for every byte of code, for a table that
+/// is mostly empty: that is what a small device cannot spare. The compact
+/// delta form in [`pos`](crate::grain::pos) is the wire form, expanded once at
+/// load.
 ///
 /// [`Positions::Stripped`] is not a degraded mode to apologize for: it is what
 /// a device runs. Errors come back carrying an instruction address instead of a
@@ -30,18 +33,32 @@ pub enum Positions {
     /// The table was never written, or was stripped before shipping.
     #[default]
     Stripped,
-    /// One entry per instruction, so a lookup is an index.
-    Dense(Box<[Position]>),
+    /// The address and position of every instruction that has one, by
+    /// address.
+    Sparse(Box<[(u32, Position)]>),
 }
 
 impl Positions {
-    /// Build from one position per instruction, dropping the table entirely if
-    /// none of them say anything.
+    /// Build from one position per code byte, keeping only those that say
+    /// something, and dropping the table entirely if none of them do.
+    #[cfg(any(test, not(feature = "no_ast")))]
     pub(crate) fn dense(positions: Vec<Position>) -> Self {
-        if positions.iter().all(|pos| pos.is_none()) {
+        Self::sparse(
+            positions
+                .into_iter()
+                .enumerate()
+                .filter(|(_, pos)| !pos.is_none())
+                .map(|(pc, pos)| (pc as u32, pos))
+                .collect(),
+        )
+    }
+
+    /// Build from the instructions that have a position, in address order.
+    fn sparse(entries: Vec<(u32, Position)>) -> Self {
+        if entries.is_empty() {
             return Self::Stripped;
         }
-        Self::Dense(positions.into_boxed_slice())
+        Self::Sparse(entries.into_boxed_slice())
     }
 
     /// The position recorded for `pc`, or `Position::NONE`.
@@ -53,7 +70,10 @@ impl Positions {
     pub fn get(&self, pc: usize) -> Position {
         match self {
             Self::Stripped => Position::NONE,
-            Self::Dense(positions) => positions.get(pc).copied().unwrap_or(Position::NONE),
+            Self::Sparse(entries) => u32::try_from(pc)
+                .ok()
+                .and_then(|pc| entries.binary_search_by_key(&pc, |&(at, _)| at).ok())
+                .map_or(Position::NONE, |i| entries[i].1),
         }
     }
 
@@ -69,14 +89,14 @@ impl Positions {
     /// Instructions with no position are skipped, which is most of them.
     #[must_use]
     pub fn to_table(&self) -> Vec<u8> {
-        let Self::Dense(positions) = self else {
+        let Self::Sparse(entries) = self else {
             return crate::grain::pos::encode(core::iter::empty());
         };
 
-        crate::grain::pos::encode(positions.iter().enumerate().filter_map(|(pc, pos)| {
+        crate::grain::pos::encode(entries.iter().filter_map(|&(pc, pos)| {
             let line = pos.line()?;
             Some((
-                pc as u32,
+                pc,
                 Site {
                     line: line as u32,
                     column: pos.position().unwrap_or(0) as u32,
@@ -85,7 +105,7 @@ impl Positions {
         }))
     }
 
-    /// Expand a compact table back to one entry per code byte.
+    /// Expand a compact table back to an entry per instruction it names.
     ///
     /// Every address has to name the *start* of an instruction in `code`.
     /// Counting how many landed in range is not enough: another program's table
@@ -107,12 +127,15 @@ impl Positions {
         // Walked, not indexed: only an address the walk arrives at begins an
         // instruction. One landing mid-instruction is as wrong as one past the
         // end, and reads as a plausible position.
-        let mut positions = vec![Position::NONE; code.len()];
+        let mut entries = Vec::with_capacity(count as usize);
         let mut matched = 0usize;
         let mut at = 0usize;
         while at < code.len() {
             if let Some(site) = crate::grain::pos::resolve(table, at as u32) {
-                positions[at] = site_to_position(site);
+                let pos = site_to_position(site);
+                if !pos.is_none() {
+                    entries.push((at as u32, pos));
+                }
                 matched += 1;
             }
             match crate::grain::bytecode::code::width(code, at) {
@@ -131,7 +154,7 @@ impl Positions {
             });
         }
 
-        Ok(Self::dense(positions))
+        Ok(Self::sparse(entries))
     }
 }
 

src/grain/bytecode/verify.rs

@@ -252,10 +252,11 @@ pub fn verify(
     //
     // Over the whole buffer at once, because every chunk shares it and an
     // instruction boundary is a property of the bytes, not of who runs them.
-    let mut starts = vec![false; code.len() + 1];
+    // One bit per byte.
+    let mut starts = vec![0u64; code.len() / 64 + 1];
     let mut at = 0usize;
     while at < code.len() {
-        starts[at] = true;
+        starts[at / 64] |= 1 << (at % 64);
         let width = code::width(code, at).ok_or(VerifyError::Undecodable { at })?;
         check_indices(at, code, pools)?;
         at += width;
@@ -296,13 +297,52 @@ struct State {
     handlers: usize,
 }
 
+/// Whether an instruction starts at `at`, by pass one's bits.
+fn starts_at(starts: &[u64], at: usize) -> bool {
+    starts
+        .get(at / 64)
+        .map_or(false, |bits| bits & (1 << (at % 64)) != 0)
+}
+
+/// Every offset `op` can jump to, onto `out`.
+fn jump_targets(op: &Op, pools: &Pools, out: &mut Vec<u32>) {
+    match *op {
+        Op::Jump(target)
+        | Op::JumpIfFalse { target }
+        | Op::JumpIfTrue { target }
+        | Op::SkipIfNotUnit { target }
+        | Op::PushHandler { target, .. }
+        | Op::IterNext { exit: target, .. } => out.push(target),
+        Op::Switch(index) => {
+            if let Some(table) = pools.switches.get(index as usize) {
+                out.extend(
+                    table
+                        .cases
+                        .iter()
+                        .flat_map(|cases| cases.values())
+                        .map(|&(target, _)| target)
+                        .chain(table.ranges.iter().map(|range| range.target))
+                        .chain(core::iter::once(table.default)),
+                );
+            }
+        }
+        _ => {}
+    }
+}
+
 /// Walk one chunk's reachable instructions, checking that every path into an
 /// instruction agrees on the stack depth.
+///
+/// Only an instruction something jumps to can be reached from more than one
+/// place: any other is reached only by falling through from the one before
+/// it, so its state follows from that one's. Recording the state at the entry
+/// and at every jump target is therefore enough to see every merge, and it
+/// costs memory in the chunk's branches rather than its length.
 fn verify_chunk(
     caps: Caps,
     code: &[u8],
     chunk: &Chunk,
-    starts: &[bool],
+    starts: &[u64],
     pools: &Pools,
 ) -> Result<u16, VerifyError> {
     let (entry, end) = (chunk.entry() as usize, chunk.end() as usize);
@@ -314,7 +354,18 @@ fn verify_chunk(
         });
     }
 
-    let mut depth_at: Vec<Option<State>> = vec![None; code.len()];
+    let mut targets = vec![entry as u32];
+    let mut at = entry;
+    while at < end {
+        let op = code::decode(code, at).ok_or(VerifyError::Undecodable { at })?;
+        jump_targets(&op, pools, &mut targets);
+        at += code::width(code, at).ok_or(VerifyError::Undecodable { at })?;
+    }
+    targets.sort_unstable();
+    targets.dedup();
+
+    // The state every path into each of `targets` arrives in, once one has.
+    let mut seen: Vec<Option<State>> = vec![None; targets.len()];
     let mut work_list = vec![(entry, State::default())];
     let mut high_water = 0usize;
 
@@ -325,16 +376,18 @@ fn verify_chunk(
 
         // Merge point: either this is the first visit, or every earlier path
         // has to have arrived in the same state.
-        match depth_at[at] {
-            Some(seen) if seen == state => continue,
-            Some(seen) => {
-                return Err(VerifyError::DepthConflict {
-                    at,
-                    expected: seen.operands,
-                    found: state.operands,
-                })
+        if let Ok(slot) = targets.binary_search(&(at as u32)) {
+            match seen[slot] {
+                Some(seen) if seen == state => continue,
+                Some(seen) => {
+                    return Err(VerifyError::DepthConflict {
+                        at,
+                        expected: seen.operands,
+                        found: state.operands,
+                    })
+                }
+                None => seen[slot] = Some(state),
             }
-            None => depth_at[at] = Some(state),
         }
 
         let depth = state.operands;
@@ -404,12 +457,19 @@ fn verify_chunk(
                     target: target as u32,
                 });
             }
-            if !starts[target] {
+            if !starts_at(starts, target) {
                 return Err(VerifyError::JumpIntoAnInstruction {
                     at,
                     target: target as u32,
                 });
             }
+            // Every edge followed here has to be one `jump_targets` found, or
+            // the instruction it lands on would go unchecked as a merge point.
+            // Refused rather than trusted, so a disagreement between the two
+            // can only reject a chunk, never pass one.
+            if targets.binary_search(&(target as u32)).is_err() {
+                return Err(VerifyError::Undecodable { at });
+            }
             work_list.push((target, state));
             Ok(())
         };

src/grain/mod.rs

@@ -209,6 +209,6 @@ pub use vm::{Fault, Vm};
 #[cfg(feature = "internals")]
 pub use program::Function;
 
-#[cfg(not(feature = "no_module"))]
+#[cfg(not(feature = "internals"))]
 #[cfg(not(feature = "no_function"))]
-pub(crate) use {program::export_functions_to_module, vm::callback_wrappers};
+pub(crate) use program::Function;

src/grain/program.rs

@@ -816,63 +816,6 @@ impl<'a> Program<'a> {
     }
 }
 
-/// Export non-private functions into a [`Module`].
-#[cfg(not(feature = "no_function"))]
-#[cfg(not(feature = "no_module"))]
-pub fn export_functions_to_module(
-    program: &SharedProgram,
-    module: &mut crate::Module,
-    env: &Shared<crate::func::EncapsulatedEnviron>,
-) {
-    program
-        .functions()
-        .iter()
-        .filter(|&f| match f.access {
-            crate::func::FnAccess::Public => true,
-            crate::func::FnAccess::Private => false,
-        })
-        .for_each(|f| {
-            let fn_name: ImmutableString =
-                program.name(f.name).expect("valid function name").into();
-            let params: crate::FnArgsVec<ImmutableString> = f
-                .params
-                .iter()
-                .map(|&idx| program.name(idx).expect("valid param name").into())
-                .collect();
-
-            let fn_def = crate::func::ScriptFuncDef {
-                name: fn_name,
-                access: f.access,
-                #[cfg(not(feature = "no_object"))]
-                this_type: f
-                    .this_type
-                    .and_then(|index| program.name(index))
-                    .map(Into::into),
-                params,
-                body: crate::func::ScriptFuncPayload::GrainVM {
-                    program: program.clone(),
-                    params: f.params.iter().copied().collect(),
-                    chunk: f.chunk,
-                    span: crate::types::Span::new(
-                        program.position(f.chunk.entry() as usize),
-                        program.position(f.chunk.end() as usize),
-                    ),
-                },
-                #[cfg(feature = "metadata")]
-                comments: <_>::default(),
-            };
-
-            let hash = module.set_script_fn(fn_def);
-
-            if let Some(crate::func::RhaiFunc::Script { env: ref mut e, .. }) =
-                module.get_script_fn_by_hash_mut(hash)
-            {
-                // Encapsulated environment
-                *e = Some(env.clone());
-            }
-        });
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;