rustdoc: Reduce allocations in a theme function

[vacuus]

str::replace allocates a new String...

This could probably be made more efficient, but I think it'd have to be clunky imperative code to achieve that.

[rust-highfive]

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

[GuillaumeGomez]

I'm really not sure it'll change anything considering how rare this code is run. However, could you benchmark this locally and show us the results please? Running a perf check on this wouldn't show anything unfortunately...

[vacuus]

For what it's worth, Godbolt reports fewer assembly language instructions for processing the string when compiled with -C opt-level=3. What should I be benchmarking? Do I need a specific tool/command? (Never done much benchmarking for anything before, sorry)

[GuillaumeGomez]

You have a book on how to run benchmarks in rust here. So pick whichever you prefer. ;)

As for what do compare: take both codes (before and after your change) and run them on a string similar to the rustdoc CSS files (or on the rustdoc CSS files directly, doesn't matter). Once done, please provide the results alongside the source code for the benchmark as well.

[vacuus]

I did the benchmarks very crudely by running cargo bench multiple times and estimating the range with the bulk of the times. Still, I think it's clear that there's a relative difference. I only changed the remove calls and not the join call for the benchmark because, for some reason, I couldn't get Cargo to use nightly*. I don't think intersperse/collect could ever be slower than collect/join, though, at least for any meaningful input.

ayu:  src/librustdoc/html/static/css/themes/ayu.css (631 lines)
dark: src/librustdoc/html/static/css/themes/dark.css (498 lines)
light: src/librustdoc/html/static/css/themes/light.css (464 lines)

current:
    dark: 109 μs
    light: 104-106 μs
    ayu: 140-150 μs
this PR (without the intersperse/collect):
    dark: 81-90 μs
    light: 77-80 μs
    ayu: 104-109 μs

use criterion::{black_box, criterion_group, criterion_main, Criterion};

pub fn criterion_benchmark(c: &mut Criterion) {
    // don't mind the naming of the benchmarks
    let mut group = c.benchmark_group("dark theme");
    // the string literal being the text of the file
    group.bench_function("new", |b| b.iter(|| new(black_box([string literal].to_string()))));
    group.finish();
}

fn old(s: String) -> String {
    s
        .trim()
        .replace('\n', " ")
        .replace('\t', " ")
        .replace('/', "")
        .replace('{', "")
        .replace('}', "")
        .split(' ')
        .filter(|s| !s.is_empty())
        .collect::<Vec<&str>>()
        .join(" ")
}

fn new(s: String) -> String {
    s
        .trim()
        .chars()
        .filter_map(|c| match c {
            '\n' | '\t' => Some(' '),
            '/' | '{' | '}' => None,
            c => Some(c),
        })
        .collect::<String>()
        .split(' ')
        .filter(|s| !s.is_empty())
        .collect::<Vec<&str>>()
        .join(" ")
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);

*I tried:

• cargo +nightly bench; Cargo gave an error saying that it's not a valid subcommand
• Making a rust-toolchain.toml
• Overriding rustup to use nightly in the directory (rustup show displays nightly as the active toolchain too)

[GuillaumeGomez]

rustup install nightly && rustup default nightly in case you want the default toolchain to be nightly. However, cargo +nightly bench should work if nightly is installed, weird...

In any case, thanks for the results, it's really great!

I'll put it in the rollup since it's in a code location where the perf change won't have any impact on the results.

@bors: r+ rollup

[bors]

📌 Commit 7feb738 has been approved by GuillaumeGomez