perf: get unpack() out of the NYI list - #269
Open
MyNameIsTrez wants to merge 1 commit into
Open
Conversation
MyNameIsTrez
force-pushed
the
fix-unpack-nyi
branch
from
August 4, 2026 15:20
e35b0e0 to
0c02ae5
Compare
MyNameIsTrez
marked this pull request as ready for review
August 5, 2026 02:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While writing a transpiler to Lua I hit a 20x performance cliff caused by
unpack.unpackis listed as2.1 stitchon tarantool's LuaJIT Not Yet Implemented page, which explains the cliff. I confirmed this with-jv.This PR resolves the cliff by making
unpackfully compiled.The original performance cliff and my workaround
Running the MRE below against the default branch (
v2.1-agentzh) 20 times produces only fast runs (~0.025s) or slow runs (~0.3s), never anything in between.The Dockerfile below reads
mre.luafrom the current working directory. Every Dockerfile in this PR description is run withdocker build -t luajit2-test . && docker run --rm -it luajit2-test.Fast runs are expected. Slow runs happen when LuaJIT blacklists
empty_fn()in response tounpackNYIs. Pass-jvtoluajitto see these sporadicblacklistedmessages.Dockerfile that runs
mre.luamre.luaTo work around this, I updated my transpiler to generate a specialized wrapper per argument count instead of forwarding arguments through
unpack. Each wrapper indexes theargstable directly and is cached, so the code generation cost is paid once while execution stays fully traceable by LuaJIT. This workaround will stay relevant for years, since many programs never update the LuaJIT version they embed.Workaround
mre.luaRunning the 32 tests I wrote for
recff_unpackDockerfile that runs
t/unpack.tits 32recff_unpacktestsIt prints this:
Confirming this fixed the original
mre.luaDockerfile that checks out this PR's branch, to demonstate the original
mre.luanow always runs fastRunning
unimutto mutation testrecff_unpackAlthough I brought line and branch coverage to 100%, I couldn't be sure I was covering every edge case, or that there were no redundant sections I could cut.
To address this, I wrote unimut (universal mutator,
pip install unimut) for this PR. It is called universal because it lets users register backends for other languages too:unimut can be run like
unimut --file src/lj_ffrecord.c --run 'make -j$(nproc) && prove t/unpack.t'. The Dockerfile below compiles with ASan and UBSan, which brings surviving mutants down from 11 to 9, and adds temporary// unimut onand// unimut offmarkers around therecff_unpackfunction this PR adds:Dockerfile that mutation tests
recff_unpackIt prints this concise diff
The 9 surviving mutants are expected. They involve checks against internal LuaJIT implementation details that Lua-level tests can't, or shouldn't, cover.
The CI already fails on the base
v2.1-agentzhbranchThe Travis CI pipeline fails on the Valgrind job, but the latest commit on the base
v2.1-agentzhbranch fails with the exact same error. This PR doesn't introduce any new test failures.