Optimize the codegen for Span::from_expansion

[Jarcho]

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]

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

[Urgau]

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

[Jarcho]

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.

[jieyouxu]

r? @petrochenkov

[petrochenkov]

Suggested change

	// Optimizes much better than `inline_ctxt` since it avoids comparing against `lo_or_index`.
	// Optimizes much better than `inline_ctxt` since it avoids comparing against `ctxt_or_parent_or_marker`.

[petrochenkov]

I wonder how to write this better so it doesn't look like magic.
It took me some time and manual inlining to understand why this works.

[petrochenkov]

Perhaps like this:

        let ctxt = match_span_kind! {
            self,
            // All branches here, except `InlineParent`, actually return `span.ctxt_or_parent_or_marker`,
            // this makes the code optimize very well by eliminating the `ctxt_or_parent_or_marker` comparison.
            InlineCtxt(span) => SyntaxContext::from_u16(span.ctxt),
            InlineParent(_span) => SyntaxContext::root(),
            PartiallyInterned(span) => SyntaxContext::from_u16(span.ctxt),
            Interned(_span) => SyntaxContext::from_u16(CTXT_INTERNED_MARKER),
        };

[petrochenkov]

In any case, let's benchmark this on rustc too.
@bors try @rust-timer queue

[bors]

⌛ Trying commit 761d0ec with merge fb65274...

[bors]

☀️ Try build successful - checks-actions
Build commit: fb65274 (fb65274cfcdf4129ff7a2c2c9c7c63b4014b82d4)

[rust-timer]

Finished benchmarking commit (fb65274): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.5%	[-2.8%, -0.3%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.5%	[-2.8%, -0.3%]	2

Max RSS (memory usage)

Results (primary 0.7%, secondary 1.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.4%	[0.4%, 6.4%]	6
Regressions ❌ (secondary)	3.2%	[2.6%, 3.7%]	2
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.4%]	8
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	0.7%	[-0.6%, 6.4%]	14

Cycles

Results (primary -0.7%, secondary -2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.4%, 0.5%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-2.6%, -0.4%]	11
Improvements ✅ (secondary)	-2.5%	[-3.0%, -2.2%]	5
All ❌✅ (primary)	-0.7%	[-2.6%, 0.5%]	13

Binary size

Results (primary -1.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Bootstrap: 770.233s -> 770.093s (-0.02%)
Artifact size: 365.54 MiB -> 365.45 MiB (-0.02%)

[Jarcho]

Went with a more verbose explanation of why this version optimizes better and why it gets the right result. Also that's a little more of a benefit than I was expecting from just rustc.