fix(parser): prevent multiple Parser instances in binary#22120
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
overlookmotel
changed the title
Parser instances in binaryParser instances in binary
overlookmotel
changed the title
Parser instances in binaryParser instances in binary
There was a problem hiding this comment.
Pull request overview
This PR addresses downstream binary size bloat caused by per-crate monomorphization of Parser<C>::parse/parse_expression after Parser became generic over ParserConfig. It does so by routing parsing through non-generic, #[inline(never)] helper functions for the known configs so the parser body is emitted once in oxc_parser and shared by consumers.
Changes:
- Add
Any-based dispatch inParser<C>::parseandParser<C>::parse_expressionto select one of three non-generic parse helpers (NoTokensParserConfig,TokensParserConfig,RuntimeParserConfig). - Introduce non-generic,
#[inline(never)]helper functions to keep the heavy parser code from being duplicated across downstream crates (even under fat LTO). - Require
ParserConfig: 'staticto supportAny-based dispatch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/oxc_parser/src/lib.rs | Adds Any-based dispatch in entry points and non-generic, #[inline(never)] helpers to reduce downstream code duplication. |
| crates/oxc_parser/src/config.rs | Updates ParserConfig trait bound to Default + 'static and documents why. |
camc314
left a comment
camc314
left a comment
There was a problem hiding this comment.
i hate this haha.
nice work!
Merge activity
|
Fixes rolldown/rolldown#9166 ### The problem #19637 made `Parser` generic over `ParserConfig`. Unfortunately this had a negative impact on Rolldown's binary size - the parser ended up taking ~1.4 MiB of `.text` in Rolldown's `.dylib`, up from ~322 KiB before. Cause is not multiple `ParserConfig`s in use - Rolldown, and all crate it depends on, only ever uses `NoTokensParserConfig`. Cause is that Rust monomorphizes generic functions per consuming crate, so each downstream crate that calls `Parser::new(...).parse()` ended up with its own private copy of the parser. None of those copies could be deduped by COMDAT (different mangled names) or by fat LTO (slightly different inlining contexts produced bit-distinct bodies). Rolldown ended up with 7 copies. ### This PR This PR solves the problem without changing the external API. `Parser<ParserConfig>` API remains as it was, but `Parser::parse` now routes through non-generic helpers - one per known config - so the parser body lives in `oxc_parser`'s rlib only once and gets shared by every consumer. The mechanism: `parse` and `parse_expression` upcast `&self.config` to `&dyn Any`, then dispatch with `Any::is::<NoTokensParserConfig>()`, `Any::is::<TokensParserConfig>()`, and `Any::downcast_ref::<RuntimeParserConfig>()`. Once monomorphized, the casts and branches are all removed by compiler, leaving just a single direct call to one of the non-generic helper functions - no wrapper symbol survives in the binary. The helpers are `#[inline(never)]` to stop fat LTO re-inlining the parser body across the rlib boundary, which would defeat the whole thing. User-defined `ParserConfig` impls fall through to a generic body in the `else` branch. Those still monomorphize per consuming crate, but that's the same cost the parser had before, just isolated to the rare custom-config case. ### Verification Have tested that this _does_ actually solve the problem in Rolldown this time: - Before: 1.4 MiB `oxc_parser` `.text`, 7 copies of `parse_statement_list_item`, dylib 24.6 MB - After: 350 KiB `oxc_parser` `.text`, 1 copy, dylib 22.9 MB Matches the pre-#19637 baseline within ~30 KiB.
7f1499e to
81e834c
Compare
Yeah, it feels wrong, and not scalable at all. Hopefully we can find a better way. But I thought this was the least disruptive solution for now. |
maybe worth making an issue in the rust repo, or perhaps reaching out on their discord (or i think they maybe use tulip) to see if this can be addressed in a better way on their end. (they may well just say this is expected behaviour, but I struggled to find docs for this). It would be nice to get it all written down somewhere for anyone else to reference in future encountering the same problem. |
Fixes rolldown/rolldown#9166
The problem
#19637 made
Parsergeneric overParserConfig. Unfortunately this had a negative impact on Rolldown's binary size - the parser ended up taking ~1.4 MiB of.textin Rolldown's.dylib, up from ~322 KiB before.Cause is not multiple
ParserConfigs in use - Rolldown, and all crate it depends on, only ever usesNoTokensParserConfig.Cause is that Rust monomorphizes generic functions per consuming crate, so each downstream crate that calls
Parser::new(...).parse()ended up with its own private copy of the parser. None of those copies could be deduped by COMDAT (different mangled names) or by fat LTO (slightly different inlining contexts produced bit-distinct bodies). Rolldown ended up with 7 copies.This PR
This PR solves the problem without changing the external API.
Parser<ParserConfig>API remains as it was, butParser::parsenow routes through non-generic helpers - one per known config - so the parser body lives inoxc_parser's rlib only once and gets shared by every consumer.The mechanism:
parseandparse_expressionupcast&self.configto&dyn Any, then dispatch withAny::is::<NoTokensParserConfig>(),Any::is::<TokensParserConfig>(), andAny::downcast_ref::<RuntimeParserConfig>(). Once monomorphized, the casts and branches are all removed by compiler, leaving just a single direct call to one of the non-generic helper functions - no wrapper symbol survives in the binary.The helpers are
#[inline(never)]to stop fat LTO re-inlining the parser body across the rlib boundary, which would defeat the whole thing.User-defined
ParserConfigimpls fall through to a generic body in theelsebranch. Those still monomorphize per consuming crate, but that's the same cost the parser had before, just isolated to the rare custom-config case.Verification
Have tested that this does actually solve the problem in Rolldown this time:
oxc_parser.text, 7 copies ofparse_statement_list_item, dylib 24.6 MBoxc_parser.text, 1 copy, dylib 22.9 MBMatches the pre-#19637 baseline within ~30 KiB.