gromandClaude Opus 5
81d96af35d
This checkout had no git history at all, so this one commit carries the whole
of the Everwood work as a single import on top of chairmanpow/benilla-octo's
`main` (706cd71). The two trees have genuinely diverged — each has files the
other does not — so this is a fork point, not a catch-up.
Today's work, which is the reason a baseline exists at all:
- Booth previews no longer cast world shadows (`NO_BOOTH_SHADOW`). Portrait
characters were entering the sun's shadow cascades, so every skinned-rig
shadow specialization compiled live mid-frame; the hitch detector logged
"main thread blocked 1316 ms" beside a `compiled LIVE` warning. This was
the Fashion/Tmog stutter.
- Per-pane dress-up: each `DressUpModel` pane assembles and stages its own
tried-on look instead of every pane mirroring the live player.
- Item template cache is append-only JSONL written on the IO pool, replacing
an 11.6 MB whole-map JSON rewrite on the main thread every 10 s.
- `WOW_SYSTEM_TIMING` rebuilt per-thread: it took 2 mutexes per system enter
and 4 per exit, costing 15 ms a frame (590 fps -> 60) and corrupting the
measurements it was taken to produce.
- The visibility-churn sweep moved to its own `WOW_FLASH_CHURN` switch, so a
frame-cost reading no longer pays 2.7 ms a frame for it.
Local state (`benilla-config/`, `logs/`, `target/`) is excluded by the
existing .gitignore; the account credential it holds is not in this commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
124 lines
7.4 KiB
Markdown
124 lines
7.4 KiB
Markdown
# benilla's `mlua-sys` fork: what differs from upstream, and how to check
|
||
|
||
Upstream: [`mlua-rs/mlua`](https://github.com/mlua-rs/mlua), MIT, crate `mlua-sys` version
|
||
`0.10.0`, the version mlua 0.11.6 resolves to. Wired in through `[patch.crates-io]` in the
|
||
workspace root.
|
||
|
||
## Why a fork exists at all
|
||
|
||
`src/lua51/compat.rs` is mlua-sys' back-port of the Lua 5.2/5.3 auxiliary library onto Lua 5.1,
|
||
derived from `lua-compat-5.3`. One of the functions it back-ports is `luaL_traceback`, and 5.3's
|
||
`luaL_traceback` names each stack frame by **searching for the frame's function by value**: the
|
||
globals table, then one level into every table in it, comparing every value with `lua_rawequal`
|
||
until it finds a match (`compat53_findfield` / `compat53_pushglobalfuncname`). That is run once per
|
||
stack level, per traceback.
|
||
|
||
mlua builds exactly that traceback for **every `Err` a Rust binding returns** — `callback_error_ext`
|
||
in mlua's `src/state/util.rs` wraps the error as `Error::CallbackError { traceback, cause }` before
|
||
re-raising it — and benilla's Lua surface is ~2,400 Rust bindings that raise by returning `Err`
|
||
(`Usage: UnitName("unit")`, `Unknown unit name: …`, the addon instruction budget).
|
||
|
||
The search's cost is the size of `_G`, and a loaded 1.12 interface's `_G` is not small. Measured by
|
||
`benilla-app`'s `addon_harness::tick_bench` on the shipped FrameXML plus the 23 addons under
|
||
`benilla-config/AddOns`:
|
||
|
||
```text
|
||
[raise] what mlua's handler would walk: 27114 globals, 83372 keys one level in, biggest _G (27114)
|
||
[raise] a binding's own `Usage:` raise, caught by Lua: 6949.2 us ← before this fork
|
||
[raise] the same raise in a bare VM (1169 globals): 83.2 us
|
||
```
|
||
|
||
**6.9 ms for one raise** (7.0–7.5 ms across runs), and a raise an addon wraps in `pcall` pays all of
|
||
it and reaches no log: `pcall` catches the error before the host ever sees it, so the only symptom
|
||
is a frame that took too long. Five such raises a frame is an entire frame budget, with a clean
|
||
error log — 44.7 ms of VM tick, measured below.
|
||
|
||
benilla already had to route around the same search once: handler and `C_Timer` dispatch go through
|
||
Lua's own `xpcall` rather than mlua's protected call, for this reason
|
||
(`benilla_ui::script::event::PROTECTED_CALL`, whose doc comment carries the measurement). That
|
||
route cannot reach `callback_error_ext`, which is inside the `lua_CFunction` mlua wraps every
|
||
binding in. A fork is the only place the cost can be removed from.
|
||
|
||
## What is not being done
|
||
|
||
benilla is not forking mlua, and not changing the error contract. The raise's **message** is
|
||
untouched: it is built by the binding, carried as the `cause`, and printed first. The traceback
|
||
still has its `stack traceback:` header, its `\n\t`-indented frame list, its `<file>:<line>:`
|
||
prefixes and its `(...tail calls...)` and `(skipping %d levels)` lines. Only how a frame's
|
||
*function* is named changes, and only for frames whose call site gives no name.
|
||
|
||
The other five Lua dialects and the whole of Luau are upstream's, untouched, including their own
|
||
copies of `compat53_findfield` (`src/lua52/compat.rs`, `src/lua53/compat.rs`,
|
||
`src/luau/compat.rs`). benilla builds `lua51` and only `lua51`; a hunk in the others would be a
|
||
hunk nobody compiles.
|
||
|
||
## The delta, and the command that proves it
|
||
|
||
One file, `src/lua51/compat.rs`. Two deletions and one changed line, all under a `BENILLA DELTA`
|
||
banner that carries the reasoning at the site:
|
||
|
||
| hunk | why |
|
||
|---|---|
|
||
| `compat53_findfield` and `compat53_pushglobalfuncname` deleted, and the `std::ffi::CStr` import with them (its only use was the `_G.` prefix strip in `compat53_pushglobalfuncname`) | the globals-table search, 6.9 ms a raise on this interface |
|
||
| `compat53_pushfuncname`'s first arm is now `*ar.namewhat != '\0'` → `function '<ar.name>'`, i.e. the name from the call site | this *is* Lua 5.1's own naming: `db_errorfb` in `third_party/lua-src/lua-5.1.5/ldblib.c:357` writes `" in function " LUA_QS` off `ar.name` under exactly that test, and searches nothing |
|
||
|
||
Upstream's remaining arms (`main chunk`, `function <%s:%d>`, `?`) are unchanged and in the same
|
||
order, so a frame the search would not have named is named exactly as before.
|
||
|
||
### Why the logged text does not move
|
||
|
||
`lua_getinfo`'s `name`/`namewhat` come from the **call site**: a binding called as a global carries
|
||
`namewhat = "global"`, `name = "UnitName"`, which is the same name the `_G` search would have
|
||
found. Upstream printed `function 'UnitName'` from the search and `global 'UnitName'` from the call
|
||
site; this fork prints `function 'UnitName'` from the call site, which is byte-identical to what
|
||
the log carried before. That matters: `benilla-app`'s `addon_harness` reads
|
||
`in function 'loadaddon'` out of a traceback to decide whose raise a demand-load's error is
|
||
(`raised_inside_a_demand_load`), and the `Usage:`/`Unknown unit name:`/instruction-budget messages
|
||
have tests pinning them.
|
||
|
||
What does change is a frame whose function is a global *and* whose call site gives no name — a tail
|
||
call, or a function handed to `pcall` by value. Those drop from `function 'Name'` to
|
||
`function <file:line>` (Lua frame) or `?` (C function), which is what Lua 5.1 and the 1.12 client's
|
||
own `debugstack` (`0x703760`, reimplemented in `benilla_ui::script::stdlib::debugstack`) both print
|
||
there, and what benilla's handler-dispatch traceback already printed.
|
||
|
||
### Verifying against upstream
|
||
|
||
```sh
|
||
# 0.10.0 is the pinned version; adjust the path if cargo's registry hash differs.
|
||
diff -r --exclude=BENILLA.md --exclude=LICENSE --exclude=Cargo.toml \
|
||
--exclude=Cargo.lock --exclude=Cargo.toml.orig --exclude='.cargo*' \
|
||
third_party/mlua-sys ~/.cargo/registry/src/*/mlua-sys-0.10.0
|
||
```
|
||
|
||
That must name exactly `src/lua51/compat.rs` and nothing else. The excludes are all bookkeeping:
|
||
`Cargo.toml` because the fork prepends a header comment to cargo's generated manifest; `LICENSE`
|
||
because the registry copy ships none and the fork carries mlua's MIT text beside the source; and
|
||
`Cargo.lock`, `Cargo.toml.orig` and `.cargo-ok`/`.cargo_vcs_info.json` because they are the
|
||
registry's own packaging artefacts, which a path dependency ignores.
|
||
|
||
## What this bought, measured
|
||
|
||
Same bench, same command (`cargo test --release -p benilla-app --lib -- --ignored --nocapture
|
||
tick_bench`), same interface:
|
||
|
||
| row | before | after |
|
||
|---|---|---|
|
||
| a binding's own `Usage:` raise, caught by Lua | 6949.2 us | **2.8 us** |
|
||
| the same raise in a bare VM (1169 globals) | 83.2 us | 2.6 us |
|
||
| `UiScript::run` of a raising chunk, through mlua | 6434.3 us | 2.3 us |
|
||
| five `pcall`'d binding raises a frame, over the whole VM tick | 45.104 ms/frame | 0.507 ms/frame |
|
||
| the same frame with the raises disarmed (the control) | 0.412 ms/frame | 0.446 ms/frame |
|
||
|
||
Two rows carry the argument. The bare-VM row is the control that names the cause: 1,169 globals
|
||
cost 83 us where 27,114 cost 6,949, a ratio of 84 against a `_G` ratio of 23 — the search is
|
||
quadratic-ish because it also walks one level in. And the five-raises row is the shape a player
|
||
actually hits: an interface that swallows five binding raises a frame spent **44.7 ms of every
|
||
frame** inside the VM tick and wrote **nothing** to any error log, which is a frame budget gone
|
||
with no symptom an instrument was looking for.
|
||
|
||
The test that fails if the search returns is
|
||
`benilla_ui::script::tests::caught_raise_cost::a_caught_binding_raise_does_not_cost_the_size_of_the_global_table`:
|
||
it measures the identical caught raise in a bare VM and in a 21,171-global one and refuses a ratio
|
||
over 4×. Reverting this fork takes it to 37.3× (3607.2 us against 96.8 us).
|