From 775e936f7d8e461b7267bb13763db37c1c45afb8 Mon Sep 17 00:00:00 2001 From: Pier-Olivier Thibault Date: Thu, 21 Jul 2022 03:19:18 -0400 Subject: [PATCH] FileHandle: Change from boxed to Arc. (#1415) * FileHandle: Change from boxed to Arc. Changing from a Box to an Arc would allow for a user of tantivy to manage file handles outside of tantivy and be able to manage their life cycle. * Fix: Rust linter --- src/directory/directory.rs | 2 +- src/directory/file_slice.rs | 17 +++++++++-------- src/directory/footer.rs | 9 +++++---- src/directory/managed_directory.rs | 4 ++-- src/directory/mmap_directory.rs | 4 ++-- src/directory/ram_directory.rs | 4 ++-- src/termdict/sstable_termdict/termdict.rs | 3 ++- 7 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/directory/directory.rs b/src/directory/directory.rs index 7c7f81f66..43d6ce5f4 100644 --- a/src/directory/directory.rs +++ b/src/directory/directory.rs @@ -111,7 +111,7 @@ pub trait Directory: DirectoryClone + fmt::Debug + Send + Sync + 'static { /// /// Users of `Directory` should typically call `Directory::open_read(...)`, /// while `Directory` implementor should implement `get_file_handle()`. - fn get_file_handle(&self, path: &Path) -> Result, OpenReadError>; + fn get_file_handle(&self, path: &Path) -> Result, OpenReadError>; /// Once a virtual file is open, its data may not /// change. diff --git a/src/directory/file_slice.rs b/src/directory/file_slice.rs index dac821f84..4bfe00e33 100644 --- a/src/directory/file_slice.rs +++ b/src/directory/file_slice.rs @@ -54,7 +54,7 @@ impl From for FileSlice where B: StableDeref + Deref + 'static + Send + Sync { fn from(bytes: B) -> FileSlice { - FileSlice::new(Box::new(OwnedBytes::new(bytes))) + FileSlice::new(Arc::new(OwnedBytes::new(bytes))) } } @@ -75,7 +75,7 @@ impl fmt::Debug for FileSlice { impl FileSlice { /// Wraps a FileHandle. - pub fn new(file_handle: Box) -> Self { + pub fn new(file_handle: Arc) -> Self { let num_bytes = file_handle.len(); FileSlice::new_with_num_bytes(file_handle, num_bytes) } @@ -83,9 +83,9 @@ impl FileSlice { /// Wraps a FileHandle. #[doc(hidden)] #[must_use] - pub fn new_with_num_bytes(file_handle: Box, num_bytes: usize) -> Self { + pub fn new_with_num_bytes(file_handle: Arc, num_bytes: usize) -> Self { FileSlice { - data: Arc::from(file_handle), + data: file_handle, range: 0..num_bytes, } } @@ -235,6 +235,7 @@ impl FileHandle for OwnedBytes { #[cfg(test)] mod tests { use std::io; + use std::sync::Arc; use common::HasLen; @@ -242,7 +243,7 @@ mod tests { #[test] fn test_file_slice() -> io::Result<()> { - let file_slice = FileSlice::new(Box::new(b"abcdef".as_ref())); + let file_slice = FileSlice::new(Arc::new(b"abcdef".as_ref())); assert_eq!(file_slice.len(), 6); assert_eq!(file_slice.slice_from(2).read_bytes()?.as_slice(), b"cdef"); assert_eq!(file_slice.slice_to(2).read_bytes()?.as_slice(), b"ab"); @@ -286,7 +287,7 @@ mod tests { #[test] fn test_slice_simple_read() -> io::Result<()> { - let slice = FileSlice::new(Box::new(&b"abcdef"[..])); + let slice = FileSlice::new(Arc::new(&b"abcdef"[..])); assert_eq!(slice.len(), 6); assert_eq!(slice.read_bytes()?.as_ref(), b"abcdef"); assert_eq!(slice.slice(1..4).read_bytes()?.as_ref(), b"bcd"); @@ -295,7 +296,7 @@ mod tests { #[test] fn test_slice_read_slice() -> io::Result<()> { - let slice_deref = FileSlice::new(Box::new(&b"abcdef"[..])); + let slice_deref = FileSlice::new(Arc::new(&b"abcdef"[..])); assert_eq!(slice_deref.read_bytes_slice(1..4)?.as_ref(), b"bcd"); Ok(()) } @@ -303,7 +304,7 @@ mod tests { #[test] #[should_panic(expected = "end of requested range exceeds the fileslice length (10 > 6)")] fn test_slice_read_slice_invalid_range_exceeds() { - let slice_deref = FileSlice::new(Box::new(&b"abcdef"[..])); + let slice_deref = FileSlice::new(Arc::new(&b"abcdef"[..])); assert_eq!( slice_deref.read_bytes_slice(0..10).unwrap().as_ref(), b"bcd" diff --git a/src/directory/footer.rs b/src/directory/footer.rs index ca673390f..81f1fb99a 100644 --- a/src/directory/footer.rs +++ b/src/directory/footer.rs @@ -156,6 +156,7 @@ impl TerminatingWrite for FooterProxy { mod tests { use std::io; + use std::sync::Arc; use common::BinarySerializable; @@ -168,7 +169,7 @@ mod tests { let footer = Footer::new(123); footer.append_footer(&mut buf).unwrap(); let owned_bytes = OwnedBytes::new(buf); - let fileslice = FileSlice::new(Box::new(owned_bytes)); + let fileslice = FileSlice::new(Arc::new(owned_bytes)); let (footer_deser, _body) = Footer::extract_footer(fileslice).unwrap(); assert_eq!(footer_deser.crc(), footer.crc()); } @@ -181,7 +182,7 @@ mod tests { let owned_bytes = OwnedBytes::new(buf); - let fileslice = FileSlice::new(Box::new(owned_bytes)); + let fileslice = FileSlice::new(Arc::new(owned_bytes)); let err = Footer::extract_footer(fileslice).unwrap_err(); assert_eq!( err.to_string(), @@ -198,7 +199,7 @@ mod tests { let owned_bytes = OwnedBytes::new(buf); - let fileslice = FileSlice::new(Box::new(owned_bytes)); + let fileslice = FileSlice::new(Arc::new(owned_bytes)); let err = Footer::extract_footer(fileslice).unwrap_err(); assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof); assert_eq!( @@ -217,7 +218,7 @@ mod tests { let owned_bytes = OwnedBytes::new(buf); - let fileslice = FileSlice::new(Box::new(owned_bytes)); + let fileslice = FileSlice::new(Arc::new(owned_bytes)); let err = Footer::extract_footer(fileslice).unwrap_err(); assert_eq!(err.kind(), io::ErrorKind::InvalidData); assert_eq!( diff --git a/src/directory/managed_directory.rs b/src/directory/managed_directory.rs index 1b7c23a05..f066d5358 100644 --- a/src/directory/managed_directory.rs +++ b/src/directory/managed_directory.rs @@ -269,9 +269,9 @@ impl ManagedDirectory { } impl Directory for ManagedDirectory { - fn get_file_handle(&self, path: &Path) -> Result, OpenReadError> { + fn get_file_handle(&self, path: &Path) -> Result, OpenReadError> { let file_slice = self.open_read(path)?; - Ok(Box::new(file_slice)) + Ok(Arc::new(file_slice)) } fn open_read(&self, path: &Path) -> result::Result { diff --git a/src/directory/mmap_directory.rs b/src/directory/mmap_directory.rs index 1c7e1bc0a..6d1239371 100644 --- a/src/directory/mmap_directory.rs +++ b/src/directory/mmap_directory.rs @@ -310,7 +310,7 @@ pub(crate) fn atomic_write(path: &Path, content: &[u8]) -> io::Result<()> { } impl Directory for MmapDirectory { - fn get_file_handle(&self, path: &Path) -> result::Result, OpenReadError> { + fn get_file_handle(&self, path: &Path) -> result::Result, OpenReadError> { debug!("Open Read {:?}", path); let full_path = self.resolve_path(path); @@ -331,7 +331,7 @@ impl Directory for MmapDirectory { }) .unwrap_or_else(OwnedBytes::empty); - Ok(Box::new(owned_bytes)) + Ok(Arc::new(owned_bytes)) } /// Any entry associated to the path in the mmap will be diff --git a/src/directory/ram_directory.rs b/src/directory/ram_directory.rs index bb9c32050..e75e45453 100644 --- a/src/directory/ram_directory.rs +++ b/src/directory/ram_directory.rs @@ -160,9 +160,9 @@ impl RamDirectory { } impl Directory for RamDirectory { - fn get_file_handle(&self, path: &Path) -> Result, OpenReadError> { + fn get_file_handle(&self, path: &Path) -> Result, OpenReadError> { let file_slice = self.open_read(path)?; - Ok(Box::new(file_slice)) + Ok(Arc::new(file_slice)) } fn open_read(&self, path: &Path) -> result::Result { diff --git a/src/termdict/sstable_termdict/termdict.rs b/src/termdict/sstable_termdict/termdict.rs index 169cedad1..29e2ffbbe 100644 --- a/src/termdict/sstable_termdict/termdict.rs +++ b/src/termdict/sstable_termdict/termdict.rs @@ -1,4 +1,5 @@ use std::io; +use std::sync::Arc; use common::BinarySerializable; use once_cell::sync::Lazy; @@ -138,7 +139,7 @@ impl TermDictionary { } pub fn from_bytes(owned_bytes: OwnedBytes) -> crate::Result { - TermDictionary::open(FileSlice::new(Box::new(owned_bytes))) + TermDictionary::open(FileSlice::new(Arc::new(owned_bytes))) } /// Creates an empty term dictionary which contains no terms.