Avoid repeated parser table decode and cut parse setup overhead (issue 630)#631
Avoid repeated parser table decode and cut parse setup overhead (issue 630)#631avityuk wants to merge 2 commits intosoftdevteam:masterfrom
Conversation
|
|
||
| /// 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> { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Agreed: ParserData would be better. If you force push that, I think we're ready to merge!
|
I have one easy comment: @ratmice does this look OK to you? |
|
I haven't yet had a chance to look, but I will try and have a gander this evening, in a couple of hours. |
|
Looks like what it says on the tin, so this looks OK to me, couldn't help but join the bikeshed a little though. |
3a033be to
30a9766
Compare
30a9766 to
dcdfda0
Compare
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 fromlrpar_modwere 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:
Box<&dyn Fn(...)>, introducing a heap allocation around an already-borrowed callback on every parse.Vec<Result<...>>and then walked it again to build the lexeme vector.This PR removes those extra costs by:
OnceLock.lrpar::ParserTableswrapper so generated code does not need to name lrtable types directlySince 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.