Repository navigation
Pass grammar locations to the generated parser as indexes - #2
manticore-projects merged 1 commit into
Conversation
The generated parser passes the grammar location of each jj_consume_token and jj_scan_token call, and of each jj_la1_loc entry, as a "line:col" string literal used in error messages. Each distinct location takes two of the at most 65535 constant pool entries of the parser class: in JSQLParser's parser, 7,436 locations take 14,872 of its 47,463 entries. The generated code now passes the 1-based index of the location in a table of strings, which is in a nested class, JJLocations, loaded with the first error message. jj_loc(int) gives the string back to the error reporting code, so the error messages don't change. jj_consume_token (int, String) is still generated for grammar code that calls it. To generate both versions of jj_consume_token, its generation moved into a method. Its body is unchanged. On JSQLParser, the parser class goes from 47,463 to 32,603 constant pool entries. Signed-off-by: minleejae <mmj9808@gmail.com>
|
Following up on the side question in JSQLParser/JSqlParser#2866: since JSQLParser's build depends on this fork and JavaCC.org is no longer responding, it might be worth hosting the fork under the JSQLParser organization. JSQLParser contributors who hit a generator limit, like the constant pool here, would find it more easily, and issues could be enabled there for reports like this one. I understand it also serves your other projects, so feel free to ignore this if it doesn't fit. |
In a perfect world with reasonable people, where everyone cooperates smoothly for the best of human mankind, your were absolutely right. I hold no power over JSQLParser upstream, there was no stable release for almost 2 years for that reason. |
|
Of course your suggestion has one huge advantage: testing! JSQLParser is probably the most sophisticated consumer of JavaCC-8 and every change should be tested against the test suite and benchmarks of JSQLParser first. |
|
Hi @manticore-projects Thanks for sharing your perspective. Your reasons for keeping the fork make sense, and I agree that testing JavaCC changes against JSQLParser’s test suite and benchmarks would be valuable. |
|
Published to SonaType Central and bumped in JSQLParser already. Thank you so much! |
The generated parser now passes its grammar locations (the
"line:col"strings used in error messages) as indexes into a table that is loaded with the first error message. On JSQLParser, the parser class goes from 47,463 to 32,603 of its 65,535 constant pool entries. Error messages are unchanged, and parsing is about 3% faster in JSQLParser's JMH benchmark.This follows up on JSQLParser/JSqlParser#2866, where @manticore-projects agreed to a generator change. Issues are disabled here, so I'm sending the PR directly.
Why
A class file holds at most 65,535 constant pool entries. JSQLParser's
CCJSqlParseris at 47,463 on current master (a30a679), up from 46,304 on 2026-10-28 and 12,266 in JSQLParser 5.3. JaCoCo already fails on it withClassTooLargeException, so JSQLParser now excludes it from coverage (JSQLParser/JSqlParser#2867). javac fails once the parser itself reaches the limit.Every
jj_consume_tokenandjj_scan_tokencall and everyjj_la1_locentry passes its grammar location as a string literal. Each distinct location takes two constant pool entries, aStringand itsUtf8: in JSQLParser's parser, 7,436 locations take 14,872 entries, 31% of the pool. The locations are only read to build error messages.In JSQLParser/JSqlParser#2866 I suggested moving the
jj_3*routines into a nested class. I tried that first. It brings the parser class to 38,744 entries, but in JSQLParser's JMH benchmark parsing gets 5–6% slower (16.50 / 16.60 / 16.75 → 17.51 / 17.69 / 17.59 ms/op), as the lookahead routines then reach the parser's fields through the outer instance. Moving the locations out saves more and leaves the parsing code as it is.Change (
ParserCodeGenerator)jj_consume_token,jj_scan_token,jj_la1_locand the_locsarrays of the token set choices take anintlocation: the 1-based index of the"line:col"string in a table, or 0 for nonejj_loc(int)returns the string fromJJLocations.LOCATIONS, a nested class that builds the table from string constants of at most 60,000 characters. The class is loaded when the first error message is builtgenerateParseException(String),jj_add_error_tokenandtrace_expectedget the strings throughjj_loc, so the messages are unchangedjj_consume_token(int, String)ERROR_REPORTINGThe generation of
jj_consume_tokenmoved into a method,emitConsumeToken, to generate both versions. Its body is unchanged.Verification
On JSQLParser master
a30a679:releasebuilt locally againstjavacc-core8.1.3.1943javacc-java8.1.3.1943releaseonce the strings are put back in place of their indexes and the intjj_consume_token,jj_locandJJLocationsare removed, apart from the checksum lineJJLocations: 43)ClassTooLargeException→ 52,363 entriesgradlew checkjj_rescan_tokenJSQLParserBenchmark(latest), 3 alternating runs of 3 forks × 5 iterationsGrammars from this repository's history (68 under
examples/,grammars/andissues/at028e00a, andissues/bas03from #1):STATIC,ERROR_REPORTING=false,CACHE_TOKENS,DEBUG_LOOKAHEAD,DEBUG_PARSER,DEPTH_LIMIT=100,JAVA_TEMPLATE_TYPE=modern,FORCE_LA_CHECK,LEGACY_EXCEPTION_HANDLING=falsereleaseonce the locations are put back (481, or 679 with the modern template), and every parser that compiles withjavac --release 8fromreleasealso compiles from this branchERROR_REPORTING=true, with and withoutSTATIC, parsing JSQLParser's 1,244 source filesrelease: 601 parsed, 643 errors with locationsissues/bas03(token set choices, replayed byjj_rescan_token)release, and the error files match the expected onesNot in this PR
jj_3*routines, given its cost. With this change, JSQLParser's parser has room for a while without it.AI assistance: prepared with Claude Code.