feat(ast_tools): pre-compile #[ast] macro output#5796
feat(ast_tools): pre-compile #[ast] macro output#5796overlookmotel wants to merge 2 commits intomainfrom
#[ast] macro output#5796Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @overlookmotel and the rest of your teammates on |
overlookmotel
force-pushed
the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
from
661db42 to
f80c7af
Compare
CodSpeed Performance ReportMerging #5796 will not alter performanceComparing Summary
|
Boshen
changed the base branch from
09-14-refactor_ast_move_functions_to_top_level_in_ast_macro
to
graphite-base/5796
Boshen
force-pushed
the
graphite-base/5796
branch
from
e02fd02 to
17cd903
Compare
Boshen
force-pushed
the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
from
f80c7af to
5fa7b5a
Compare
Boshen
force-pushed
the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
from
5fa7b5a to
502fd10
Compare
overlookmotel
force-pushed
the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
from
502fd10 to
a40e8c7
Compare
overlookmotel
force-pushed
the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
from
a40e8c7 to
90746ab
Compare
| /// TokenTree::Group(Group::new(Delimiter::Parenthesis, args)) | ||
| /// ].into_iter()) | ||
| /// ``` | ||
| macro_rules! code { |
There was a problem hiding this comment.
This is super smart! I like what I'm seeing👍
There was a problem hiding this comment.
Thanks. But unfortunately it's not moving the needle on performance.
I suspect (but don't know) that creating TokenTrees and TokenStreams is inherently expensive due to how proc macros interface with the compiler. There may just be no way to make proc macros cheap.
Or maybe the #[ast] macro just isn't that costly in the first place, so optimizing it is never going to pay dividends.
Thanks for taking a look. I'm going to pause on this though for now as am planning to spend this week focused on transformer. Must... not... get... distracted...
By the way, these 2 articles were the inspiration. They do a very good job of explaining how proc macros work internally. I always thought it involved serialization, but it turns out not:
There was a problem hiding this comment.
Or maybe the #[ast] macro just isn't that costly in the first place, so optimizing it is never going to pay dividends.
I think this is more likely; No matter how much proc macro overhead costs us, If we do enough work in a macro eventually caching it would be faster. I suspect we have a better chance of getting some juice out of this approach in #4112 However it might be a wild goose chase.
And thanks for the links, They seem really interesting especially the second part(gave them a cursory look), I'll give them a read.
|
Close as stale. Feel free to revive this. |
overlookmotel
deleted the
09-14-feat_ast_tools_pre-compile_ast_macro_output
branch
|
Archived on overlookmotel/precompile-ast-macro-output branch. |
3 participants
Work in progress.
Generate code for
#[ast]macro with anoxc_ast_toolsgenerator.oxc_ast_macroscrate now has no dependencies (nosynetc).The guts of it is the
code!macro. This is the same asquote!, except it evaluates to aTokenStreamof code to create theTokenStreamwhichquote!evaluates to.Unfortunately I'm not seeing an improvement in compile times, because other dependencies of
oxc_astalso usesyn, so it doesn't drop from the dependency graph.The generated code is 3000 lines, and could likely be shrunk down, which also could help.The generated code is now 600 lines, and now there is a 5% reduction in debug build time foroxc_astcrate - but that's still a disappointingly small improvement.Am opening this PR not necessarily with the intent of merging it, but to discuss whether this is maybe a useful approach. @rzvxa would appreciate your feedback.
Note: This technique would allow
#[generate_derive]to inject the derivedimplnext to the type def, same as#[derive]does, which would solve the problem of types/fields needing to bepub(crate).