Skip to content

Optimize the codegen for Span::from_expansion #140485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Apr 29, 2025

See https://godbolt.org/z/bq65Y6bc4 for the difference. the new version is less than half the number of instructions.

Also tried fully writing the function by hand:

sp.ctxt_or_parent_or_marker != 0
        && (
            sp.len_with_tag_or_marker == BASE_LEN_INTERNED_MARKER 
            || sp.len_with_tag_or_marker & PARENT_TAG == 0
        )

But that was no better than this PR's current use of match_span_kind.

@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 29, 2025
@rust-log-analyzer

This comment has been minimized.

@Urgau
Copy link
Member

Urgau commented Apr 29, 2025

Is this function this hot that it needs micro-optimized? Have you benchmark it on real world case?

@Jarcho
Copy link
Contributor Author

Jarcho commented Apr 29, 2025

There are more than 300 call sites (most from clippy) and it ends up being called more than once per expression while linting (in clippy). It's definitely used a lot and frequently called.

@Jarcho Jarcho force-pushed the from_expansion_opt branch from e332f70 to 761d0ec Compare April 29, 2025 20:48
@jieyouxu
Copy link
Member

r? @petrochenkov

@rustbot rustbot assigned petrochenkov and unassigned jieyouxu Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants