Use Seek.stream_position instead of seek(0).

[PJB3005]

BufReader can cache the result of stream_position() so it's fast, whereas seek(0) discards the read buffer and passes through straight to the OS. This drastically worsens the efficiency of loading performance when using BufReader (or anything else to avoid all these tiny reads going straight to the kernel).

[zamazan4ik]

I am a little wondered why in previous versions, seek(0) was used. Seems really strange to me

[PJB3005]

stream_position() wasn't added until Rust 1.51, to my massive surprise. So that's probably why.

[zamazan4ik]

By the way, do you have some exact numbers regarding performance before and after the fix? I am just interested to see actual difference :)

[zamazan4ik]

Thanks for the PR!

[PJB3005]

I don't have the most scientific benchmark setup here because I just ran some ETW profiles when I noticed my zip files (like ~50 MB, ~13k mostly-tiny files) took way longer to open than they should on debug mode, then cross-referenced the code with the amount of seek calls and yep.

I just made a quick and dirty Read + Seek impl that doesn't pass through io::SeekFrom::Current(0) to the base stream:

struct FTellStream<R> {
    inner: R,
}

impl<R: Read + Seek> FTellStream<R> {
    pub fn new(inner: R) -> Self {
        FTellStream { inner }
    }
}

impl<R: Read + Seek> Read for FTellStream<R> {
    fn read(&mut self, buf: &mut [u8]) -> Result<usize, io::Error> {
        self.inner.read(buf)
    }

    fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
        self.inner.read_exact(buf)
    }
}

impl<R: Read + Seek> Seek for FTellStream<R> {
    fn seek(&mut self, from: io::SeekFrom) -> Result<u64, io::Error> {
        if from == io::SeekFrom::Current(0) {
            return self.inner.stream_position();
        }

        self.inner.seek(from)
    }
}

Without, doing this on these zip files:

        let a = Instant::now();
        let file = fs::File::open(zip_path).unwrap();
        let mut zip = ZipArchive::new( BufReader::new(file)).unwrap();
        let b = Instant::now();

        println!("{}, {:?}", zip.len(), b.duration_since(a));

I get like (on release, debug the gap is even wider):

17092, 85.636ms
17092, 86.8534ms
17092, 84.1522ms
17076, 82.9949ms
17092, 84.8181ms
17092, 84.4906ms

With:

        let a = Instant::now();
        let file = fs::File::open(zip_path).unwrap();
        let mut zip = ZipArchive::new(FTellStream::new(BufReader::new(file))).unwrap();
        let b = Instant::now();

        println!("{}, {:?}", zip.len(), b.duration_since(a));

17092, 38.4624ms
17092, 38.2349ms
17092, 40.5351ms
17076, 38.3368ms
17092, 37.9675ms
17092, 37.6783ms

Should be noted that this is on Windows, for however much that matters.

[PJB3005]

I was benchmarking for #303 and the seek calls still go through to the OS even with BufReader, so using this FTellStream which caches the stream position would improve perf even more. I'm not sure this is something the library can properly implement itself though so I'll just leave it here:

struct FTellStream<R> {
    inner: R,
    position: u64,
}

impl<R: Read + Seek> FTellStream<R> {
    pub fn new(mut inner: R) -> Self {
        let position = inner.stream_position().unwrap();
        FTellStream { inner, position }
    }
}

impl<R: Read + Seek> Read for FTellStream<R> {
    fn read(&mut self, buf: &mut [u8]) -> Result<usize, io::Error> {
        let read = self.inner.read(buf)?;

        self.position += read as u64;

        return Ok(read);
    }

    fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
        self.inner.read_exact(buf)?;

        self.position += buf.len() as u64;

        Ok(())
    }
}

impl<R: Read + Seek> Seek for FTellStream<R> {
    fn seek(&mut self, from: io::SeekFrom) -> Result<u64, io::Error> {
        if from == io::SeekFrom::Current(0) {
            return self.stream_position();
        }

        self.position = self.inner.seek(from)?;
        Ok(self.position)
    }

    fn stream_position(&mut self) -> io::Result<u64> {
        Ok(self.position)
    }
}