Skip to content

Avoid repeated parser table decode and cut parse setup overhead (issue 630)#631

Open
avityuk wants to merge 2 commits intosoftdevteam:masterfrom
avityuk:issue-630-parse-performance
Open

Avoid repeated parser table decode and cut parse setup overhead (issue 630)#631
avityuk wants to merge 2 commits intosoftdevteam:masterfrom
avityuk:issue-630-parse-performance

Conversation

@avityuk
Copy link
Copy Markdown

@avityuk avityuk commented Apr 28, 2026

While profiling a workload that parses many small inputs in a tight loop, I found two sources of avoidable per-parse overhead in lrpar.

First, generated parse() functions from lrpar_mod were calling _reconstitute(__GRM_DATA, __STABLE_DATA) on every invocation, which meant re-decoding the grammar and state table every time even though RTParserBuilder::new only borrows them. This change caches the reconstituted tables in generated code and reuses them across calls.

Second, there were a couple of smaller setup costs in lrpar::parser:

  • parser::token_cost was stored as Box<&dyn Fn(...)>, introducing a heap allocation around an already-borrowed callback on every parse.
  • parse_map and parse_actions collected the lexer iterator into Vec<Result<...>> and then walked it again to build the lexeme vector.

This PR removes those extra costs by:

  • caching generated parser tables behind a OnceLock.
  • introducing an opaque lrpar::ParserTables wrapper so generated code does not need to name lrtable types directly
  • storing token_cost as a borrowed callback rather than boxing it
  • collecting lexemes in one pass

Since the time spent in _reconstitute is proportional to grammar size, this change is particularly impactful there.

However, even on a very small grammar, such as calc_actions example, these changes brought a tight-loop parse benchmark down from roughly ~2.06 µs/parse to ~0.80 µs/parse.

Comment thread lrpar/src/lib/ctbuilder.rs Outdated

/// Bundles the parser tables (`YaccGrammar` + `StateTable`) so that generated parsers can hold
/// them in a `OnceLock` without naming `lrtable` directly.
pub struct ParserTables<StorageT: Eq + Hash> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been chewing on the right name for this. It's not "tables", quite. I think I might go for ParserBundle. I also think we should "nodoc" this struct, because we don't want users relying on this part of the API (ideally they won't even know it exists).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I kind of like is something along the lines of ParserData, or ParserDataBundle, but I'm not too picky, and ParserBundle seems okay too.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with ParserBundle and added [doc(hidden)] (I assume that's what "nodoc" is?).

Let me know if you change your mind with naming. ParserData doesn't sound bad either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed: ParserData would be better. If you force push that, I think we're ready to merge!

@ltratt
Copy link
Copy Markdown
Member

ltratt commented Apr 28, 2026

I have one easy comment: @ratmice does this look OK to you?

@ratmice
Copy link
Copy Markdown
Collaborator

ratmice commented Apr 28, 2026

I haven't yet had a chance to look, but I will try and have a gander this evening, in a couple of hours.

@ratmice
Copy link
Copy Markdown
Collaborator

ratmice commented Apr 28, 2026

Looks like what it says on the tin, so this looks OK to me, couldn't help but join the bikeshed a little though.

@avityuk avityuk force-pushed the issue-630-parse-performance branch from 3a033be to 30a9766 Compare April 29, 2026 05:01
@avityuk avityuk force-pushed the issue-630-parse-performance branch from 30a9766 to dcdfda0 Compare April 29, 2026 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants