[rustc_span][perf] Remove unnecessary string joins and allocs.

[ttsugriy]

Comparing vectors of string parts yields the same result but avoids unnecessary join and potential allocation for resulting String. This code is cold so it's unlikely to have any measurable impact, but considering but since it's also simpler, why not? :)

[rustbot]

r? @fee1-dead

(rustbot has picked a reviewer for you, use r? to override)

[lqd]

Do you know if the call site immediately above

rust/compiler/rustc_span/src/edit_distance.rs

Lines 240 to 250 in 7a5d2d0

	fn find_match_by_sorted_words(iter_names: &[Symbol], lookup: &str) -> Option<Symbol> {
	iter_names.iter().fold(None, |result, candidate| {
	if sort_by_words(candidate.as_str()) == sort_by_words(lookup) {
	Some(*candidate)
	} else {
	result
	}
	})
	}
	fn sort_by_words(name: &str) -> String {

is actually calling sort_by_words once per iteration for the same lookup input or is it LICMed away ?

[ttsugriy]

Do you know if the call site immediately above

rust/compiler/rustc_span/src/edit_distance.rs

Lines 240 to 250 in 7a5d2d0

	fn find_match_by_sorted_words(iter_names: &[Symbol], lookup: &str) -> Option<Symbol> {
	iter_names.iter().fold(None, |result, candidate| {
	if sort_by_words(candidate.as_str()) == sort_by_words(lookup) {
	Some(*candidate)
	} else {
	result
	}
	})
	}
	fn sort_by_words(name: &str) -> String {

is actually calling sort_by_words once per iteration for the same lookup input or is it LICMed away ?

I assumed that this fold would be desugared into a loop and compiler wouldn't have much difficulties to hoist this computation out of the loop. But apparently I was wrong according to https://godbolt.org/z/frs8Kj1rq
In particular the left version uses:

.LBB16_3:
        mov     rbx, qword ptr [r13]
        mov     r14, qword ptr [r13 + 8]
        lea     rdi, [rsp + 40]
        mov     rsi, rbx
        mov     rdx, r14
        call    example::sort_by_words
        lea     rdi, [rsp + 64]
        mov     rsi, qword ptr [rsp + 8]
        mov     rdx, qword ptr [rsp + 16]
        call    example::sort_by_words
        mov     rdi, qword ptr [rsp + 40]
        mov     rdx, qword ptr [rsp + 56]
        mov     rsi, qword ptr [rsp + 64]
        cmp     rdx, qword ptr [rsp + 80]
        mov     qword ptr [rsp + 32], rdi
        mov     qword ptr [rsp + 24], rsi
        jne     .LBB16_5
        call    qword ptr [rip + bcmp@GOTPCREL]
        test    eax, eax
        sete    al
        mov     dword ptr [rsp + 4], eax
        mov     rsi, qword ptr [rsp + 72]
        test    rsi, rsi
        jne     .LBB16_8
        jmp     .LBB16_9

2 calls to sort_by_words and the right side only a single one:

.LBB16_3:
        mov     r13, qword ptr [r15]
        mov     r14, qword ptr [r15 + 8]
        lea     rdi, [rsp + 64]
        mov     rsi, r13
        mov     rdx, r14
        call    example::sort_by_words
        mov     rdi, qword ptr [rsp + 64]
        mov     rdx, qword ptr [rsp + 16]
        cmp     qword ptr [rsp + 80], rdx
        mov     qword ptr [rsp + 32], rdi
        jne     .LBB16_5
        mov     rsi, qword ptr [rsp + 8]
        call    qword ptr [rip + bcmp@GOTPCREL]
        test    eax, eax
        sete    bpl
        mov     rsi, qword ptr [rsp + 72]
        test    rsi, rsi
        jne     .LBB16_8
        jmp     .LBB16_9

Amusingly the total number of assembly lines is the same, but one of the versions clearly seems to be more performant :)

In any case, since hosting is not related to current change, I'll send a separate PR once this one is merged.

[fee1-dead]

@bors r+ rollup

[bors]

📌 Commit f74eee2 has been approved by fee1-dead

It is now in the queue for this repository.

[ttsugriy]

btw, just for fun, I've also wrote a benchmark and got

     Running benches/sort_by_words_benchmark.rs (target/release/deps/sort_by_words_benchmark-5adee33fa37ce7e2)
Benchmarking multiply add/original: Collecting 100 samples in estimated 5.0006 s (27M iteratio
multiply add/original   time:   [172.85 ns 177.30 ns 182.83 ns]
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
Benchmarking multiply add/proposed: Collecting 100 samples in estimated 5.0000 s (61M iteratio
multiply add/proposed   time:   [81.106 ns 81.605 ns 82.261 ns]
Found 12 outliers among 100 measurements (12.00%)
  5 (5.00%) high mild
  7 (7.00%) high severe

although the numbers may be different for different strings.