Inline fail! causes significant slowdown in write()

[mahkoh]

Consider the following snippet:

#[inline(never)]
fn fail() {
    fail!("assertion failed");
}

impl<'a, W: Writer> Writer for UnsafeWriter<'a, W> {
    fn write(&mut self, buf: &[u8]) -> IoResult<()> {
        let dst = self.buf.mut_slice_from(self.pos);
        if buf.len() > dst.len() {
            fail();
        }
        unsafe {
            copy_nonoverlapping_memory(dst.as_mut_ptr(), buf.as_ptr(), buf.len())
        }
        self.pos += buf.len();
        Ok(())
    }

    fn flush(&mut self) -> IoResult<()> {
        self.flush_buf().and_then(|()| self.inner.flush())
    }
}

Inlining fail() causes significant slowdown in the following program

https://gist.github.com/anonymous/48ba255d89c2f9bf636c

Inlined:

real    0m3.256s
user    0m3.237s
sys 0m0.020s

Non-inlined:

real    0m1.562s
user    0m1.537s
sys 0m0.023s

Compiled with -O. The program is tested with time ./cat -v /tmp/blabla.txt > /dev/null where the text file are 100MB of random data.

Note that core::slice::MutableVector::copy_memory has the form

#[inline]
    unsafe fn copy_memory(self, src: &[T]) {
        let len_src = src.len();
        assert!(self.len() >= len_src);
        ptr::copy_nonoverlapping_memory(self.as_mut_ptr(), src.as_ptr(), len_src)
    }

where assert! expands to something similar to the inlined fail!() version above. This function is used by some Writers.

cc @cmr

[mahkoh]

perf non-inlined

~/rust/coreutils/cat$ perf stat ./cat -v /tmp/blabla.txt > /dev/null

 Performance counter stats for './cat -v /tmp/blabla.txt':

       1598.236550      task-clock (msec)         #    0.999 CPUs utilized          
                12      context-switches          #    0.008 K/sec                  
                26      cpu-migrations            #    0.016 K/sec                  
               313      page-faults               #    0.196 K/sec                  
     3,356,501,774      cycles                    #    2.100 GHz                    
     1,378,688,552      stalled-cycles-frontend   #   41.08% frontend cycles idle   
     1,012,720,553      stalled-cycles-backend    #   30.17% backend  cycles idle   
     4,485,975,285      instructions              #    1.34  insns per cycle        
                                                  #    0.31  stalled cycles per insn
     1,435,550,790      branches                  #  898.209 M/sec                  
        76,588,047      branch-misses             #    5.34% of all branches        

       1.599360146 seconds time elapsed

perf inlined:

~/rust/coreutils/cat$ perf stat ./cat -v /tmp/blabla.txt > /dev/null

 Performance counter stats for './cat -v /tmp/blabla.txt':

       3283.608533      task-clock (msec)         #    1.001 CPUs utilized          
                 7      context-switches          #    0.002 K/sec                  
                21      cpu-migrations            #    0.006 K/sec                  
               336      page-faults               #    0.102 K/sec                  
     7,105,046,317      cycles                    #    2.164 GHz                    
     1,729,009,226      stalled-cycles-frontend   #   24.33% frontend cycles idle   
     1,119,307,210      stalled-cycles-backend    #   15.75% backend  cycles idle   
    12,545,235,483      instructions              #    1.77  insns per cycle        
                                                  #    0.14  stalled cycles per insn
     3,403,847,620      branches                  # 1036.618 M/sec                  
        78,237,041      branch-misses             #    2.30% of all branches        

       3.281030203 seconds time elapsed

---

nm non-inlined:

~/rust/coreutils/cat$ nm cat| grep "UnsafeWriter.*write"
~/rust/coreutils/cat$ 

nm inlined:

~/rust/coreutils/cat$ nm cat| grep "UnsafeWriter.*write"
000000000040c110 t _ZN40UnsafeWriter$LT$$x27a$C$$x20W$GT$.Writer5write21h130431865473282111524v0.0E

[huonw]

BTW, theoretically that .write should be returning an Err rather than failing to allow callers to handle the error.

[mahkoh]

In this particular program we always call unwrap() after write(). In fact: I'd rather replace all failing and unwinding by abort().

For comparison:

        if buf.len() > dst.len() {
            return Err(std::io::IoError { kind: std::io::OtherIoError, desc: "", detail: None });     
   }

real    0m1.670s
user    0m1.650s
sys 0m0.017s

---

        if buf.len() > dst.len() {
            return Err(std::io::standard_error(std::io::OtherIoError)); 
   }

real    0m1.790s
user    0m1.783s
sys 0m0.007s

---

        if buf.len() > dst.len() {
            unsafe { std::intrinsics::abort(); }
        }

real    0m1.675s
user    0m1.643s
sys 0m0.030s

---

        if buf.len() > dst.len() {
            unsafe { libc::exit(1); }
        }

real    0m1.567s
user    0m1.537s
sys 0m0.027s

The numbers are somewhat stable.

[emberian]

cc @alexcrichton @thestinger

[mahkoh]

With the currently nightly:

real    0m1.665s
user    0m1.643s
sys 0m0.017s

Inlined:

real    0m1.716s
user    0m1.673s
sys 0m0.037s

[thestinger]

See #17386

[thestinger]

Nominating because fail!() is a major performance issue and presents a backwards compatibility problem. I don't think it makes sense for failure to involve string formatting and injecting line numbers (which are usually useless, due to lack of context).

[nikomatsakis]

Updated title to be more precise.

[pnkfelix]

Assigning P-low, not 1.0.

(We should also investigate whether this benchmark still has the same performance problems, since there were some discussion that some of these paths have been improved.)