Performance regression in tight loop since rust 1.25

[pedrocr]

I've finally gotten around to doing some proper benchmarking of rust versions for my crate:

http://chimper.org/rawloader-rustc-benchmarks/

As can be seen in the graph on that page there's a general performance improvement over time but there are some very negative outliers. Most (maybe all) of them seem to be very simple loops that decode packed formats. Since rust 1.25 those are seeing 30-40% degradations in performance. I've extracted a minimal test case that shows the issue:

fn decode_12le(buf: &[u8], width: usize, height: usize) -> Vec<u16> {
  let mut out: Vec<u16> = vec![0; width*height];

  for (row, line) in out.chunks_mut(width).enumerate() {
    let inb = &buf[(row*width*12/8)..];

    for (o, i) in line.chunks_mut(2).zip(inb.chunks(3)) {
      let g1: u16 = i[0] as u16;
      let g2: u16 = i[1] as u16;
      let g3: u16 = i[2] as u16;

      o[0] = ((g2 & 0x0f) << 8) | g1;
      o[1] = (g3 << 4) | (g2 >> 4);
    }
  }
  out
}

fn main() {
  let width = 5000;
  let height = 4000;

  let buffer: Vec<u8> = vec![0; width*height*12/8];
  
  for _ in 0..100 {
    decode_12le(&buffer, width, height);
  }
}

Here's a test run on my machine:

$ rustc +1.24.0 -C opt-level=3 bench_decode.rs 
$ time ./bench_decode 

real	0m4.817s
user	0m3.581s
sys	0m1.236s
$ rustc +1.25.0 -C opt-level=3 bench_decode.rs 
$ time ./bench_decode 

real	0m6.263s
user	0m5.067s
sys	0m1.196s

[pedrocr]

godbolt shows quite a big diff between 1.24 and 1.25:

https://godbolt.org/g/fbpuHp

[pedrocr]

I've scripted the checking of this across versions and the regression is present all the way through to nightly:

packed12le : 1.24.1  BASE 3.53
packed12le : 1.25.0  FAIL 4.84 (+37%)
packed12le : 1.26.2  FAIL 4.80 (+35%)
packed12le : 1.27.2  FAIL 4.81 (+36%)
packed12le : 1.28.0  FAIL 4.87 (+37%)
packed12le : 1.29.2  FAIL 4.77 (+35%)
packed12le : 1.30.0  FAIL 4.83 (+36%)
packed12le : beta    FAIL 4.83 (+36%)
packed12le : nightly FAIL 4.95 (+40%)

The 35-40% increase in runtime is very consistent.

[pedrocr]

The regression is still present in 1.31 and all the way to current nightly:

packed12le : 1.24.1  BASE 3.51
packed12le : 1.25.0  FAIL 4.84 (+37%)
packed12le : 1.26.2  FAIL 4.83 (+37%)
packed12le : 1.27.2  FAIL 4.80 (+36%)
packed12le : 1.28.0  FAIL 4.86 (+38%)
packed12le : 1.29.2  FAIL 4.98 (+41%)
packed12le : 1.30.1  FAIL 5.00 (+42%)
packed12le : 1.31.0  FAIL 4.90 (+39%)
packed12le : beta    FAIL 4.88 (+39%)
packed12le : nightly FAIL 4.92 (+40%)

The same ~40% regression seen in the minimal test case is also seen in the full benchmark:

http://chimper.org/rawloader-rustc-benchmarks/version-1.31.0.html
(see the bottom of the page)

[bluss]

@pedrocr Thanks for careful benchmarks. I'd suspect that the zip specialization for chunks mut is causing this, it was introduced between 1.24 and 1.25 in PR #47142

It's not far fetched that it's not actually an optimization for these iterators, and that the implementation should be revisited.

[bluss]

What's the performance if you compare this version with something based on the newer chunks_exact/_mut ?

[pedrocr]

Using chunks_exact_mut the regression is completely reverted and converted into a ~10% improvement:

packed12le : 1.24.1  BASE 3.53
packed12le : 1.25.0  FAIL 4.93 (+39%)
packed12le : 1.26.2  FAIL 4.82 (+36%)
packed12le : 1.27.2  FAIL 4.85 (+37%)
packed12le : 1.28.0  FAIL 4.93 (+39%)
packed12le : 1.29.2  FAIL 4.94 (+39%)
packed12le : 1.30.1  FAIL 5.03 (+42%)
packed12le : 1.31.0  OK   3.19 (-9%)
packed12le : beta    OK   3.18 (-9%)
packed12le : nightly OK   3.08 (-12%)

Starting with 1.31 the chunks_exact_mut is used instead of chunks_mut. At some point I'll probably make 1.31 (and Rust 2018) a new baseline required version to be able to use this. But it's probably nice to fix this regression anyway as the exact versions are not always usable.

[bluss]

@pedrocr just to clarify, did you update all occurrences of chunks/_mut to be the exact version?

[pedrocr]

@bluss Both the inner and outer loop. Here's the full code:

fn decode_12le(buf: &[u8], width: usize, height: usize) -> Vec<u16> {
  let mut out: Vec<u16> = vec![0; width*height];

  for (row, line) in out.chunks_exact_mut(width).enumerate() {
    let inb = &buf[(row*width*12/8)..];

    for (o, i) in line.chunks_exact_mut(2).zip(inb.chunks(3)) {
      let g1: u16 = i[0] as u16;
      let g2: u16 = i[1] as u16;
      let g3: u16 = i[2] as u16;

      o[0] = ((g2 & 0x0f) << 8) | g1;
      o[1] = (g3 << 4) | (g2 >> 4);
    }
  }
  out
}

fn main() {
  let width = 5000;
  let height = 4000;

  let mut buffer: Vec<u8> = vec![0; width*height*12/8];
  // Make sure we don't get optimized out by writing some data into the buffer
  for (i, val) in buffer.chunks_mut(1).enumerate() {
    val[0] = i as u8;
  }
  
  for _ in 0..100 {
    decode_12le(&buffer, width, height);
  }
}

I've also initialized the buffer with some data to avoid optimization. I think that became needed after one of the LLVM upgrades since 1.25.

[bluss]

There's still a chunks in there, why not try with it exact too?

[pedrocr]

I did, and it doesn't make a difference, it's just the initialization which doesn't take much time. I didn't change that one because it's not in the code that's actually being benchmarked, but it doesn't really make a difference.

[bluss]

Maybe change the chunks to chunks exact on this line. I'm just curious.

for (o, i) in line.chunks_exact_mut(2).zip(inb.chunks(3)) {

[pedrocr]

Ah, right, missed that one. I'll check.