copy_tv() and clear_tv() can be improved for scalar values - #20787
copy_tv() and clear_tv() can be improved for scalar values#20787thinca wants to merge 2 commits into
Conversation
Problem: copy_tv() and clear_tv() make an out-of-line call and run a type
switch even for scalar values (number/bool/special/float), whose
copy or clear is trivial; the overhead adds up in value-churn-heavy
Vim9 code.
Solution: Inline the scalar fast path of copy_tv()/clear_tv() at the call site,
falling back to the renamed out-of-line copy_tv_inner()/
clear_tv_inner() for every refcounted or allocating type.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: thinca <thinca@gmail.com>
inline_funcs.h is reached through vim.h, so every object depends on it. Add it to the manually-maintained header lists (VIM_H_DEPENDENCIES in Makefile, the INCL lists in Make_mvc.mak and Make_cyg_ming.mak, and the per-object list in Make_vms.mms) and to the "### Dependencies:" block in Makefile. Kept as a separate commit from the source change: it is mechanical build bookkeeping, and the Makefile dependency block is normally regenerated by "make depend". Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: thinca <thinca@gmail.com>
|
I had Clause Code review this PR. I have excluded any points that differ from my own opinion. CorrectnessThe scalar path is equivalent to the current implementation. In The scalar arms in the *_inner() functions become dead code
Having the same rule in two places, where one copy can never be executed, is a Reading the benchmark
The scalar workload is a loop of For example, how about running benchmarks with DetailsThe line length of the comment added to vim.h exceeds 80 columns. The name Using |
Summary
Every value in Vim is a tagged
typval_T, and moving values around — onto and off the Vim9 execution stack, into and out of lists/dicts, throughforloops, on function return — goes throughcopy_tv()/clear_tv(), each running a typeswitch. For the common scalar types (number/bool/special/float) the real work is a trivial value copy or a no-op, so the function call plus the switch dominate the cost.This makes
copy_tv()/clear_tv()thinstatic inlinewrappers (newsrc/inline_funcs.h) that handle the scalar types at the call site and fall back to the out-of-linecopy_tv_inner()/clear_tv_inner()for every refcounted or allocating type. The public names are unchanged, so every caller benefits with no call-site churn; observable behaviour is identical.Two design notes:
typval_T(fromstructs.h) and the*_inner()prototypes (fromproto.h), so they must be included afterproto.h;macros.his included too early. A small dedicated header keepsvim.hshort, but these two definitions could just as well sit inline invim.hafterproto.h— happy to inline them instead if you prefer.copy_tv()/clear_tv()would drop the switch but keep the call; inlining at the call site drops both — which is why the wall-clock gain below exceeds the instruction-count gain.The build-dependency bookkeeping for the new header is split into a second commit, since it is mechanical and is normally regenerated by
make depend.Benchmark
scalar (best case): −10% instructions, −23% wall-clock. container (worst case): unchanged (+1%
Ir). Binary: +0.6%.Base —
masterat commit6f02e5cd7c27fc098d0b8dfec99542c7663a807e— vs patched, measured on GitHub Actions. callgrind instruction-reads (Ir) are deterministic and runner-independent; wall-clock is the minimum of 3 native runs (informational — noisy on shared runners).Irbase → patchedIrΔBoth workloads produce bit-identical results on the base and patched builds, confirming the change is semantics-preserving.
copy_tv()/clear_tv()are exercised throughout the existing test suite, so no new test is added.For the scalar workload the wall-clock gain (−23%) is larger than the
Irgain (−10%): removing the call and the type switch also helps branch prediction and the instruction cache, whichIralone does not capture. The container workload is effectively unchanged — its sub-1%Irincrease is the expected cost of the extra scalar-type check before the non-scalar fallback. Its wall-clock delta is within run-to-run noise on the shared runner, where the deterministicIris the reliable signal.This is a general, low-risk constant-factor win: scalars are a common case whose copy or clear is trivial, so paying a call plus a type switch for them is avoidable overhead. It does not touch the interpreter dispatch loop, so it helps all value-churn-heavy Vim9 code rather than being a redesign.
Binary size
Inlining the scalar fast-path at every call site grows the binary a little (Ubuntu 24.04 CI runner, x86-64, gcc
-O2without-g, base → patched):vimbinaryBenchmark workloads (vim9script)
Iris measured withvalgrind --tool=callgrind --dump-instr=no --branch-sim=noon an-O2 -gbuild; binary size and wall-clock use an-O2(no-g) build. Driven byci/bench/run_typval_bench.sh <base-sha> <patched-sha>.scalarbench.vim— scalar / best case:containerbench.vim— container / worst case:AI-assisted; the change is disclosed via the
Co-Authored-Bytrailer on the commit, per CONTRIBUTING "Using AI".