Also hash spans inside the same file as relative.

[cjgillot]

Spans that have a parent and that are contained in that parent are hashed as relative to the start position of their parent.

This works well for function bodies, but does not handle attributes.

This PR proposes to extend relative hashing when the span and the parent are in the same file.

[cjgillot]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 3a47352 with merge df67202...

[bors]

☀️ Try build successful - checks-actions
Build commit: df67202 (df6720223021f98b2dc8ea94b69f1991f056f3bf)

[cjgillot]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 3481584 with merge 5426bf8...

[bors]

☀️ Try build successful - checks-actions
Build commit: 5426bf8 (5426bf86cd14955ed64c175a325655d9b0b3f691)

[cjgillot]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 0dddae1 with merge 9b70448...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9b70448 (9b704482b8f77bc0b05b3cad1e80cb11fe41e40f)

[rust-timer]

Finished benchmarking commit (9b70448): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.3%]	29
Regressions ❌ (secondary)	0.3%	[0.1%, 0.5%]	35
Improvements ✅ (primary)	-6.1%	[-12.6%, -0.5%]	6
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.8%	[-12.6%, 0.3%]	35

Max RSS (memory usage)

Results (primary 0.1%, secondary 4.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.8%	[0.7%, 2.8%]	3
Regressions ❌ (secondary)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-1.7%	[-1.7%, -1.6%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-1.7%, 2.8%]	6

Cycles

Results (primary -9.8%, secondary -3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.8%	[-10.6%, -9.3%]	3
Improvements ✅ (secondary)	-3.9%	[-4.5%, -3.3%]	2
All ❌✅ (primary)	-9.8%	[-10.6%, -9.3%]	3

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 465.532s -> 464.536s (-0.21%)
Artifact size: 374.73 MiB -> 374.69 MiB (-0.01%)

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

[fee1-dead]

r? compiler

[lcnr]

r? compiler

[compiler-errors]

I'm not totally convinced this is beneficial. It may just be that our incremental tests are not robust enough to demonstrate the benefit, but right now it looks like it only improves the behavior of one type of test.

Do you want to share some words summarizing your thoughts about the perf results, @cjgillot?

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[cjgillot]

I'm not totally convinced this is beneficial. It may just be that our incremental tests are not robust enough to demonstrate the benefit, but right now it looks like it only improves the behavior of one type of test.

The goal is to accelerate the incr-patched test case: code that just moved because of an unrelated change. In this idea, this PR is a success. OTOH, the regression on incr-unchanged test case is unfortunate, because Span::hash_stable is very very hot.

This is a trade-off. I'm inclined to try making the incr-patched test case faster, as that is the core target of incr-comp. I'm open to discuss it.

[Kobzol]

We definitely do not have a lot of interesting incr patched examples in our suite, so it's possible that this might be more useful than it first seems, but already the >10% win on nalgebra seems really good. Stuff randomly moving and then having to be recompiled is personally a big annoyance for me, and if this can help avoiding that, I'm all for landing it, even if it causes small regressions for incr-unchanged.

[Kobzol]

It's interesting though, I checked both nalgebra and diesel, and both add a println! call at the top (around the 100/200 line mark) of a ~2000 line file. So the incr patched case looks similar.