Don't deep-clone the generated impl in builtin derives [-0.02%, below significance] - #66
Draft
xmakro wants to merge 1 commit into
Draft
Don't deep-clone the generated impl in builtin derives [-0.02%, below significance]#66xmakro wants to merge 1 commit into
xmakro wants to merge 1 commit into
Conversation
Attaching the forwarded lint attributes cloned the entire freshly generated derive impl -- every generated method body -- just to override the attrs field of an item we already own, and cloned the attribute list twice in the process. Extend the attributes in place instead. Attribute order and content are unchanged, and ast::Item's remaining fields are copied identically (the item comes from cx.item, which hardcodes DUMMY_NODE_ID and tokens: None).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attaching the forwarded lint attributes cloned the entire freshly generated derive impl — every generated method body, which for a
Debugderive on a wide struct is substantial AST — purely to override theattrsfield of an item already owned. It also cloned the attribute list twice, the second copy being discarded immediately by the struct-update syntax. This fires for every#[derive]on every type.Equivalence: attribute order and content are identical (own attrs first, then the filtered forwarded ones).
ast::Item's other fields are copied identically — the item comes fromcx.item, which hardcodesDUMMY_NODE_IDandtokens: None, so no fresh NodeId was ever being minted and there is no token cache to desync.Honest measurement: -0.02% geomean, zero significant cells. Algorithmically strictly less work, but not visible on this benchmark set. Included for the reduction in allocation and AST copying rather than the number.
Measured in isolation against a clean base (ThinLTO stage2, glibc, instructions:u, full scenario, 5 crates x 3 profiles = 15 cells). This change was measured alone, not as part of a batch.
Reviewed adversarially before submission; findings that survived are reflected in the code and described above.