Repository navigation
Make Grain function loading lazy, tighten up routines - #1192
ImTheSquid wants to merge 9 commits into
Conversation
Not sure what changed other than using a bit vector instead of an array of |
…ests Grain functions and plugin manifests are now both sources in one LazyFunctions list, so a Module has one field for functions looked up only when called, and the Grain-only module helpers exist only with the grain feature.
Every statement emits a positioned marker under debugging, so the table outgrows the code it describes on a loop-heavy script.
|
BTW, have you benchmarked this? How much more memory can it save? This would make Grain really light-weight I predict... |
Only Grain calls it, so it was dead code without the feature.
combine and combine_flatten remove the registered functions the incoming Grain functions take precedence over, so lookups fall through to them; fill_with skips the incoming functions this module's Grain functions already hold. Neither registers Grain functions up front any more.
Module::merge_filtered cloned every function across when the target had none, so AST::clone_functions_only_filtered kept every function.
A function pointer linked to a script function short-circuits through Module::get_script_fn_by_hash, which saw only registered functions, so a callback into a Grain closure always took the slow path. It now looks in the Grain functions too, and returns None for a non-script function instead of panicking. Each lookup also hashed every function's name again, so a callback whose operators miss the Grain module paid for that on every call. The hashes are now made on the first lookup and shared by a module's library and its exports: 8 bytes per function.
BenchmarksCompared against the merge base (
|
| µs/run | vs base | |
|---|---|---|
| base | 351.5 | |
| before the fixes | ~402 | +13% |
| fast path + hash table | 364.1 | +3.6% |
With the fast path alone, grain_bench callbacks came down from 8.7 to 7.9 ms; the hash table accounts for the rest.
The remaining gap is the lazy-source check on each of those uncached misses. Final grain_bench runs (noisy, Spotlight was indexing): VM callbacks 7.3–7.7 ms on the baseline, 7.6–7.7 ms on the branch. Every other case is within noise of the baseline. The three cases below their floor (script fn calls, native function calls, primes) are below it on the baseline too.
There was a problem hiding this comment.
So basically we avoid building an entire module hashmap of all Grain functions for callbacks, deferring the building a list of their hashes during the first lookup?
The reason why we need these hashes is because callbacks cannot be cached so this lookup happen every time, basically.
LGTM at this point, but can this feature be opt-in or opt-out? Like a new ..._with_lazy_callbacks instead of replacing the original API.
That's because I'd assume the typical Rhai script won't have that many functions to cause a material saving in runtime memory, so some users may want to just build the module in the first place as lazy loading won't save much.
This PR addresses a few problems: