From 662706607b9d235f06567eb9cc2ccd26df85a725 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Tue, 20 Oct 2020 22:36:57 +0300 Subject: [PATCH] melib: remove memmap dependency It's unmaintained, and the IO performance gains are negligible --- Cargo.lock | 23 ------------ melib/Cargo.toml | 5 +-- melib/src/backends/maildir.rs | 17 ++++++--- melib/src/backends/maildir/backend.rs | 38 +++++++++++-------- melib/src/backends/maildir/stream.rs | 12 +++--- melib/src/backends/mbox.rs | 54 +++++++++++++++------------ 6 files changed, 72 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 616c34d1a..b8c358098 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -618,16 +618,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" -[[package]] -name = "fs2" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" -dependencies = [ - "libc", - "winapi 0.3.9", -] - [[package]] name = "fsevent" version = "0.4.0" @@ -1110,7 +1100,6 @@ dependencies = [ "isahc", "libc", "libloading", - "memmap", "native-tls", "nix", "nom", @@ -1133,18 +1122,6 @@ version = "2.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3728d817d99e5ac407411fa471ff9800a778d88a24685968b36824eaf4bee400" -[[package]] -name = "memmap" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46f3c7359028b31999287dae4e5047ddfe90a23b7dca2282ce759b491080c99b" -dependencies = [ - "fs2", - "kernel32-sys", - "libc", - "winapi 0.2.8", -] - [[package]] name = "memoffset" version = "0.5.5" diff --git a/melib/Cargo.toml b/melib/Cargo.toml index f16b63faa..f21232e75 100644 --- a/melib/Cargo.toml +++ b/melib/Cargo.toml @@ -23,7 +23,6 @@ bitflags = "1.0" crossbeam = "0.7.2" data-encoding = "2.1.1" encoding = "0.2.33" -memmap = { version = "0.5.2", optional = true } nom = { version = "5.1.1" } indexmap = { version = "^1.5", features = ["serde-1", ] } @@ -61,8 +60,8 @@ http = ["isahc"] http-static = ["isahc", "isahc/static-curl"] imap_backend = ["tls"] jmap_backend = ["http", "serde_json"] -maildir_backend = ["notify", "memmap"] -mbox_backend = ["notify", "memmap"] +maildir_backend = ["notify"] +mbox_backend = ["notify"] notmuch_backend = [] smtp = ["tls", "base64"] sqlite3 = ["rusqlite", ] diff --git a/melib/src/backends/maildir.rs b/melib/src/backends/maildir.rs index 70494ff58..86b4cd4f3 100644 --- a/melib/src/backends/maildir.rs +++ b/melib/src/backends/maildir.rs @@ -31,11 +31,10 @@ use crate::email::Flag; use crate::error::{MeliError, Result}; use crate::shellexpand::ShellExpandTrait; pub use futures::stream::Stream; - -use memmap::{Mmap, Protection}; use std::collections::hash_map::DefaultHasher; use std::fs; use std::hash::{Hash, Hasher}; +use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -45,7 +44,7 @@ pub struct MaildirOp { hash_index: HashIndexes, mailbox_hash: MailboxHash, hash: EnvelopeHash, - slice: Option, + slice: Option>, } impl Clone for MaildirOp { @@ -92,10 +91,16 @@ impl MaildirOp { impl<'a> BackendOp for MaildirOp { fn as_bytes(&mut self) -> ResultFuture> { if self.slice.is_none() { - self.slice = Some(Mmap::open_path(self.path(), Protection::Read)?); + let file = std::fs::OpenOptions::new() + .read(true) + .write(false) + .open(&self.path())?; + let mut buf_reader = BufReader::new(file); + let mut contents = Vec::new(); + buf_reader.read_to_end(&mut contents)?; + self.slice = Some(contents); } - /* Unwrap is safe since we use ? above. */ - let ret = Ok((unsafe { self.slice.as_ref().unwrap().as_slice() }).to_vec()); + let ret = Ok(self.slice.as_ref().unwrap().as_slice().to_vec()); Ok(Box::pin(async move { ret })) } diff --git a/melib/src/backends/maildir/backend.rs b/melib/src/backends/maildir/backend.rs index 422b1d75e..c5b5d96fb 100644 --- a/melib/src/backends/maildir/backend.rs +++ b/melib/src/backends/maildir/backend.rs @@ -27,7 +27,6 @@ use crate::error::{ErrorKind, MeliError, Result}; use crate::shellexpand::ShellExpandTrait; use futures::prelude::Stream; -use memmap::{Mmap, Protection}; extern crate notify; use self::notify::{watcher, DebouncedEvent, RecursiveMode, Watcher}; use std::time::Duration; @@ -241,6 +240,7 @@ impl MailBackend for MaildirType { Ok(Box::pin(async move { let thunk = move |sender: &BackendEventConsumer| { debug!("refreshing"); + let mut buf = Vec::with_capacity(4096); let mut path = path.clone(); path.push("new"); for d in path.read_dir()? { @@ -278,10 +278,10 @@ impl MailBackend for MaildirType { } (*map).insert(hash, PathBuf::from(&file).into()); } - if let Ok(mut env) = Envelope::from_bytes( - unsafe { &Mmap::open_path(&file, Protection::Read)?.as_slice() }, - Some(file.flags()), - ) { + let mut reader = io::BufReader::new(fs::File::open(&file)?); + buf.clear(); + reader.read_to_end(&mut buf)?; + if let Ok(mut env) = Envelope::from_bytes(buf.as_slice(), Some(file.flags())) { env.set_hash(hash); mailbox_index .lock() @@ -371,6 +371,7 @@ impl MailBackend for MaildirType { Ok(Box::pin(async move { // Move `watcher` in the closure's scope so that it doesn't get dropped. let _watcher = watcher; + let mut buf = Vec::with_capacity(4096); loop { match rx.recv() { /* @@ -410,6 +411,7 @@ impl MailBackend for MaildirType { pathbuf.as_path(), &cache_dir, file_name, + &mut buf, ) { mailbox_index .lock() @@ -464,6 +466,7 @@ impl MailBackend for MaildirType { pathbuf.as_path(), &cache_dir, file_name, + &mut buf, ) { mailbox_index .lock() @@ -482,14 +485,14 @@ impl MailBackend for MaildirType { } }; let new_hash: EnvelopeHash = get_file_hash(pathbuf.as_path()); + let mut reader = io::BufReader::new(fs::File::open(&pathbuf)?); + buf.clear(); + reader.read_to_end(&mut buf)?; if index_lock.get_mut(&new_hash).is_none() { debug!("write notice"); - if let Ok(mut env) = Envelope::from_bytes( - unsafe { - &Mmap::open_path(&pathbuf, Protection::Read)?.as_slice() - }, - Some(pathbuf.flags()), - ) { + if let Ok(mut env) = + Envelope::from_bytes(buf.as_slice(), Some(pathbuf.flags())) + { env.set_hash(new_hash); debug!("{}\t{:?}", new_hash, &pathbuf); debug!( @@ -611,6 +614,7 @@ impl MailBackend for MaildirType { dest.as_path(), &cache_dir, file_name, + &mut buf, ) { mailbox_index .lock() @@ -701,6 +705,7 @@ impl MailBackend for MaildirType { dest.as_path(), &cache_dir, file_name, + &mut buf, ) { mailbox_index .lock() @@ -747,6 +752,7 @@ impl MailBackend for MaildirType { dest.as_path(), &cache_dir, file_name, + &mut buf, ) { mailbox_index .lock() @@ -1330,6 +1336,7 @@ fn add_path_to_index( path: &Path, cache_dir: &xdg::BaseDirectories, file_name: PathBuf, + buf: &mut Vec, ) -> Result { debug!("add_path_to_index path {:?} filename{:?}", path, file_name); let env_hash = get_file_hash(path); @@ -1344,11 +1351,10 @@ fn add_path_to_index( map.len() ); } - //Mmap::open_path(self.path(), Protection::Read)? - let mut env = Envelope::from_bytes( - unsafe { &Mmap::open_path(path, Protection::Read)?.as_slice() }, - Some(path.flags()), - )?; + let mut reader = io::BufReader::new(fs::File::open(&path)?); + buf.clear(); + reader.read_to_end(buf)?; + let mut env = Envelope::from_bytes(buf.as_slice(), Some(path.flags()))?; env.set_hash(env_hash); debug!( "add_path_to_index gen {}\t{}", diff --git a/melib/src/backends/maildir/stream.rs b/melib/src/backends/maildir/stream.rs index 8fd59fc9b..be750f725 100644 --- a/melib/src/backends/maildir/stream.rs +++ b/melib/src/backends/maildir/stream.rs @@ -25,8 +25,7 @@ use core::future::Future; use core::pin::Pin; use futures::stream::{FuturesUnordered, StreamExt}; use futures::task::{Context, Poll}; -use memmap::{Mmap, Protection}; -use std::io::{self}; +use std::io::{self, Read}; use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; use std::result; @@ -113,6 +112,7 @@ impl MaildirStream { let len = chunk.len(); let size = if len <= 100 { 100 } else { (len / 100) * 100 }; let mut local_r: Vec = Vec::with_capacity(chunk.len()); + let mut buf = Vec::with_capacity(4096); for c in chunk.chunks(size) { let map = map.clone(); for file in c { @@ -146,10 +146,10 @@ impl MaildirStream { let map = map.entry(mailbox_hash).or_default(); (*map).insert(env_hash, PathBuf::from(file).into()); } - match Envelope::from_bytes( - unsafe { &Mmap::open_path(&file, Protection::Read)?.as_slice() }, - Some(file.flags()), - ) { + let mut reader = io::BufReader::new(fs::File::open(&file)?); + buf.clear(); + reader.read_to_end(&mut buf)?; + match Envelope::from_bytes(buf.as_slice(), Some(file.flags())) { Ok(mut env) => { env.set_hash(env_hash); mailbox_index.lock().unwrap().insert(env_hash, mailbox_hash); diff --git a/melib/src/backends/mbox.rs b/melib/src/backends/mbox.rs index a00af89a5..07a5dda64 100644 --- a/melib/src/backends/mbox.rs +++ b/melib/src/backends/mbox.rs @@ -30,7 +30,6 @@ use crate::email::*; use crate::error::{MeliError, Result}; use crate::get_path_hash; use crate::shellexpand::ShellExpandTrait; -use memmap::{Mmap, Protection}; use nom::bytes::complete::tag; use nom::character::complete::digit1; use nom::combinator::map_res; @@ -41,8 +40,7 @@ use self::notify::{watcher, DebouncedEvent, RecursiveMode, Watcher}; use std::collections::hash_map::{DefaultHasher, HashMap}; use std::fs::File; use std::hash::Hasher; -use std::io::BufReader; -use std::io::Read; +use std::io::{BufReader, Read}; use std::os::unix::io::AsRawFd; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -164,7 +162,7 @@ pub struct MboxOp { path: PathBuf, offset: Offset, length: Length, - slice: Option, + slice: std::cell::RefCell>>, } impl MboxOp { @@ -172,7 +170,7 @@ impl MboxOp { MboxOp { hash, path: path.to_path_buf(), - slice: None, + slice: std::cell::RefCell::new(None), offset, length, } @@ -181,28 +179,38 @@ impl MboxOp { impl BackendOp for MboxOp { fn as_bytes(&mut self) -> ResultFuture> { - if self.slice.is_none() { - self.slice = Some(Mmap::open_path(&self.path, Protection::Read)?); + if self.slice.get_mut().is_none() { + let file = std::fs::OpenOptions::new() + .read(true) + .write(false) + .open(&self.path)?; + get_rw_lock_blocking(&file); + let mut buf_reader = BufReader::new(file); + let mut contents = Vec::new(); + buf_reader.read_to_end(&mut contents)?; + *self.slice.get_mut() = Some(contents); } - /* Unwrap is safe since we use ? above. */ - let ret = Ok((unsafe { - &self.slice.as_ref().unwrap().as_slice()[self.offset..self.offset + self.length] - }) - .to_vec()); + let ret = Ok(self.slice.get_mut().as_ref().unwrap().as_slice() + [self.offset..self.offset + self.length] + .to_vec()); Ok(Box::pin(async move { ret })) } fn fetch_flags(&self) -> ResultFuture { let mut flags = Flag::empty(); - let file = std::fs::OpenOptions::new() - .read(true) - .write(true) - .open(&self.path)?; - get_rw_lock_blocking(&file); - let mut buf_reader = BufReader::new(file); - let mut contents = Vec::new(); - buf_reader.read_to_end(&mut contents)?; - let (_, headers) = parser::headers::headers_raw(contents.as_slice())?; + if self.slice.borrow().is_none() { + let file = std::fs::OpenOptions::new() + .read(true) + .write(false) + .open(&self.path)?; + get_rw_lock_blocking(&file); + let mut buf_reader = BufReader::new(file); + let mut contents = Vec::new(); + buf_reader.read_to_end(&mut contents)?; + *self.slice.borrow_mut() = Some(contents); + } + let slice_ref = self.slice.borrow(); + let (_, headers) = parser::headers::headers_raw(slice_ref.as_ref().unwrap().as_slice())?; if let Some(start) = headers.find(b"Status:") { if let Some(end) = headers[start..].find(b"\n") { let start = start + b"Status:".len(); @@ -780,7 +788,7 @@ impl MailBackend for MboxType { let mailbox_path = mailboxes.lock().unwrap()[&mailbox_hash].fs_path.clone(); let file = std::fs::OpenOptions::new() .read(true) - .write(true) + .write(false) .open(&mailbox_path)?; get_rw_lock_blocking(&file); let mut buf_reader = BufReader::new(file); @@ -852,7 +860,7 @@ impl MailBackend for MboxType { let mailbox_hash = get_path_hash!(&pathbuf); let file = match std::fs::OpenOptions::new() .read(true) - .write(true) + .write(false) .open(&pathbuf) { Ok(f) => f,