Swap antlr4rust for antlr-rust-runtime - #35
Open
tinovyatkin wants to merge 1 commit into
Open
Conversation
Ports the parser layer from antlr4rust 0.5.2 to antlr-rust-runtime 0.23.0.
All 753 tests pass: the same golden .avpr/.avsc output compared against
avro-tools, and the same insta snapshots apart from the five noted below.
Parsing is faster, and the margin grows with input size. Measured with
hyperfine per BENCHMARKING.md, both binaries in one invocation, 40 runs
after 5 warmup, on synthetic inputs built by the "multiply named types"
method from simple.avdl:
fixture size antlr4rust this wall user CPU
10x 21 KB 16.5 ms 14.1 ms 1.17x 1.59x
20x 41 KB 21.9 ms 17.0 ms 1.29x 1.76x
50x 102 KB 33.1 ms 20.9 ms 1.58x 2.15x
100x 204 KB 52.0 ms 28.3 ms 1.84x 2.38x
Wall-clock carries fixed process startup that dominates small files, so
user CPU is the cleaner parse-cost signal: 7.0 -> 4.4 ms at 21 KB rising
to 32.6 -> 13.7 ms at 204 KB. Marginal parse cost falls from roughly
0.19 to 0.06 ms/KB. Every fixture's output was checked semantically
identical with jq -S.
Regeneration no longer needs Java, the antlr4rust fork's ANTLR JAR, or
git. antlr4-rust-gen ships inside the runtime crate, so
scripts/regenerate-antlr.sh reads the version cargo resolved and installs
that exact version into a project-local root before running it over
Idl.g4. A published version is immutable, so regeneration is
byte-reproducible by construction rather than by tracking a fork's
release asset. The property you preserved by checking in generated code —
downstream users need no Java — now covers grammar changes too.
Error spans stayed byte-exact for the miette underlines. The runtime
hands error listeners a resolved UTF-8 byte span for lexer diagnostics as
well as parser ones, so CollectingErrorListener no longer carries a copy
of the source text, converts line/column into an offset by hand, or
recovers a token's width by scanning the diagnostic message for a quoted
run. That last heuristic was silently producing single-character spans;
two snapshots improve because an unterminated string literal now
underlines the whole literal instead of pointing one caret at its opening
quote.
Deeply nested input no longer overflows the stack: rule recursion runs on
a segmented stack, regression-tested at 10k nesting.
Snapshot changes, five in total:
- error_missing_enum_commas, error_semicolons_in_enum,
error_semicolon_replaced_with_comma (tests/error_reporting.rs):
recovery is more precise on malformed input. `enum E { A B C }` was
one "unexpected }" at EOF and is now three errors, the first
correctly at B.
- unterminated_string_in_annotation, unterminated_string_in_default_value
(src/reader.rs): the wider underline described above.
No golden compiler output changed; only diagnostics on already-invalid
input became more specific.
Also fixes a pre-existing flake in tests/cli.rs that is worth having
independently of this port. supports-color, which miette delegates to,
checks FORCE_COLOR and CLICOLOR_FORCE before testing whether the stream
is a TTY, so a shell exporting either made the binary emit ANSI escapes
through a pipe and two snapshot assertions failed. NO_COLOR alone cannot
fix it because the force variables are checked first. avdl_cmd() now
clears them and sets NO_COLOR, pinning the decision instead of inheriting
it; verified deterministic under CLICOLOR_FORCE=1, FORCE_COLOR=1,
FORCE_COLOR=3 and a cleaned environment.
Review orientation: src/generated/ is now two files emitted by
antlr4-rust-gen. Typed contexts are lightweight borrows over a flat CST
rather than Rc trees, so reader.rs walker signatures take &TokenStore
instead of the token-stream generic; function structure and comments are
otherwise unchanged, so it reads as a mechanical port. Outside
src/generated/, the diff is reader.rs, doc_comments.rs,
regenerate-antlr.sh, tests/cli.rs and Cargo.{toml,lock}.
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.
This replaces antlr4rust, both the codegen and the runtime, with antlr-rust-runtime. Grammar untouched,
src/generated/regenerated,reader.rsadapted to the new context types.Disclosure up front: I maintain it. Judge it on the diff.
You filed antlr4rust #35, #36 and #37 profiling this tool, so you already know where its parse time goes. That is why I thought this was worth your time rather than just mine.
Same output. All 753 tests pass. Golden
.avpr/.avscstill match avro-tools, and I diffed every fixture's output withjq -Sbesides. Five snapshots changed, all error reporting on already-invalid input, itemised with causes in the commit message. Nothing about valid input moved.Faster, and it scales. hyperfine per your
BENCHMARKING.md, both binaries in one invocation, 40 runs after 5 warmup, synthetic inputs by the multiply-named-types method:Marginal parse cost drops from roughly 0.19 to 0.06 ms/KB. On #36 you said your case is a binary handed a handful of files at a time, so first-parse cost is what matters: that is the 21 KB row, about 1.6x on CPU.
You have probably already noticed
Idl.g4is ambiguous atnullableTypeandresultType, since every primitive keyword is also a valid identifier, and the grammar concedes as much in a comment. Both runtimes pay full-context prediction there, a couple of thousand retries on the largest fixture. I made that cheaper, not gone. Only a grammar change would.No Java to regenerate.
antlr4-rust-genships in the runtime crate, soregenerate-antlr.shreads the version cargo resolved, installs exactly that, and runs it overIdl.g4. A Rust toolchain is the only prerequisite. Published versions are immutable, so regeneration is byte-reproducible.Better spans, less code. Error listeners get a resolved UTF-8 byte span, for lexer diagnostics as well as parser ones.
CollectingErrorListenerno longer carries a copy of the source, no longer converts line/column by hand, and no longer recovers a token's width by scanning the diagnostic text for a quoted run. That last heuristic was quietly producing one-character spans; unterminated string literals now underline the whole literal.Reviewing:
src/generated/is two files now, and typed contexts are borrows over a flat CST rather thanRctrees, soreader.rssignatures take&TokenStore. Structure and comments are otherwise untouched, it should read mechanically. Outside generated code the diff isreader.rs,doc_comments.rs,regenerate-antlr.sh,tests/cli.rs,Cargo.{toml,lock}.