fix(vag_vehicle): unzip without overflowing gopher-lua's value stack - #142
Merged
Merged
Conversation
A dataset of more than a few kB failed on a box with "registry overflow". FTW runs gopher-lua, whose table.concat puts every item of the range on a value stack of about 5,000 slots, and the unzip joined its whole output in one call. It now joins at most 256 items per call. The tests run C Lua, which has no such limit, so they passed. The VAG harness now refuses table.concat ranges of 2,000 items or more, and a 360 kB fixture makes the unzip flush. Put back the old join and the test fails on a 98,346-item concat. Run in FTW's own host (NewLuaDriver, real json_decode and telemetry store, HTTP stubbed), 100 kB of JSON took 55 ms per poll and 1 MB took 0.6 s on an Apple M-series core. A poll has 10 seconds and a Raspberry Pi is several times slower, so the dataset cap drops from 4 MiB to 2 MiB. Version 0.1.1, since 0.1.0 is published. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
1 participant
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.
Follow-up to #141.
vag_vehicle0.1.0 fails on a real FTW box for any dataset larger than a few kB. This fixes it as 0.1.1.What was wrong
FTW runs its Lua drivers in gopher-lua. Its
table.concatputs every item of the range on a value stack of about 5,000 slots, so a long range fails withregistry overflow. The unzip joined its whole output in one call, 98,346 items for a 360 kB file. The first version from the Cursor agent did the same withtable.concat(out).The repository's tests run C Lua 5.5, which has no such limit, so they passed.
Fix
table.concatcall, in a small tree.table.concatranges of 2,000 items or more, like gopher-lua would. A new 360 kB fixture makes the unzip flush. With the old join put back, the test fails on a 98,346-item concat.Evidence from FTW's own host
I loaded the driver with FTW's
NewLuaDriver(real gopher-lua,json_decode,emitand telemetry store), and stubbed onlyhost.http_get:That was an Apple M-series core. A poll has 10 seconds and a Raspberry Pi is several times slower, hence the 2 MiB cap. The size of a real All Data file is still unknown.
Checks
make test-driver ID=vag_vehicle: 36 passed, 16 skippedmake check: 4731 passed, 913 skippedOther drivers may carry the same C Lua versus gopher-lua gap; that is a separate question for the harness as a whole.
🤖 Generated with Claude Code