Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize EndianReader to use ptr/len #302

Merged
merged 1 commit into from
May 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ coveralls = { repository = "gimli-rs/gimli" }
arrayvec = { version = "0.4.6", default-features = false }
byteorder = { version = "1.0", default-features = false }
fallible-iterator = { version = "0.1.4", default-features = false }
stable_deref_trait = "1.0.0"

[dev-dependencies]
crossbeam = "0.3.2"
Expand Down
107 changes: 65 additions & 42 deletions src/endian_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ use endianity::Endianity;
use parser::{Error, Result};
use rc::Rc;
use reader::Reader;
use stable_deref_trait::CloneStableDeref;
use std::fmt::Debug;
use std::mem;
use std::ops::{Deref, Index, Range, RangeFrom, RangeTo};
use std::slice;
use std::str;
use string::String;
use Arc;
Expand Down Expand Up @@ -100,10 +102,14 @@ pub type EndianArcSlice<Endian> = EndianReader<Endian, Arc<[u8]>>;
/// impl Deref for ArcMmapFile {
/// type Target = [u8];
/// fn deref(&self) -> &[u8] {
/// &*self
/// &self.0
/// }
/// }
///
/// // These are both valid for any `Rc` or `Arc`.
/// unsafe impl gimli::StableDeref for ArcMmapFile {}
/// unsafe impl gimli::CloneStableDeref for ArcMmapFile {}
///
/// /// A `gimli::Reader` that is backed by an `mmap`ed file!
/// pub type MmapFileReader<Endian> = gimli::EndianReader<Endian, ArcMmapFile>;
/// # fn test(_: &MmapFileReader<gimli::NativeEndian>) { }
Expand All @@ -112,7 +118,7 @@ pub type EndianArcSlice<Endian> = EndianReader<Endian, Arc<[u8]>>;
pub struct EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
range: SubRange<T>,
endian: Endian,
Expand All @@ -121,18 +127,18 @@ where
impl<Endian, T1, T2> PartialEq<EndianReader<Endian, T2>> for EndianReader<Endian, T1>
where
Endian: Endianity,
T1: Deref<Target = [u8]> + Clone + Debug,
T2: Deref<Target = [u8]> + Clone + Debug,
T1: CloneStableDeref<Target = [u8]> + Debug,
T2: CloneStableDeref<Target = [u8]> + Debug,
{
fn eq(&self, rhs: &EndianReader<Endian, T2>) -> bool {
self.range.bytes() == rhs.range.bytes()
self.bytes() == rhs.bytes()
}
}

impl<Endian, T> Eq for EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
}

Expand All @@ -143,59 +149,80 @@ where
// `self.endian`. Splitting the sub-range out from the endian lets us work
// around this, making it so that only the `self.range` borrow is held active,
// not all of `self`.
//
// This also serves to encapsulate the unsafe code concerning `CloneStableDeref`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also comment about how we must keep a handle to bytes around since the ptr is only valid while we still hold the bytes (eg because holding the bytes holds a refcount for us).

// The `bytes` member is held so that the bytes live long enough, and the
// `CloneStableDeref` ensures these bytes never move. The `ptr` and `len`
// members point inside `bytes`, and are updated during read operations.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
struct SubRange<T>
where
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
bytes: T,
start: usize,
end: usize,
ptr: * const u8,
len: usize,
}

impl<T> SubRange<T>
where
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
#[inline]
fn new(bytes: T) -> Self {
let ptr = bytes.as_ptr();
let len = bytes.len();
SubRange { bytes, ptr, len }
}

#[inline]
fn bytes(&self) -> &[u8] {
&self.bytes[self.start..self.end]
// Safe because `T` implements `CloneStableDeref`, `bytes` can't be modified,
// and all operations that modify `ptr` and `len` ensure they stay in range.
unsafe { slice::from_raw_parts(self.ptr, self.len) }
}

#[inline]
fn len(&self) -> usize {
self.end - self.start
self.len
}

#[inline]
fn find(&self, byte: u8) -> Option<usize> {
self.bytes().iter().position(|x| *x == byte)
fn truncate(&mut self, len: usize) {
assert!(len <= self.len);
self.len = len;
}

#[inline]
fn skip(&mut self, len: usize) {
assert!(len <= self.len);
self.ptr = unsafe { self.ptr.offset(len as isize) };
self.len -= len;
}

#[inline]
fn read_slice(&mut self, len: usize) -> Result<&[u8]> {
if self.len() < len {
Err(Error::UnexpectedEof)
} else {
let start = self.start;
self.start += len;
Ok(&self.bytes[start..start + len])
// Same as for `bytes()`.
let bytes = unsafe { slice::from_raw_parts(self.ptr, len) };
self.skip(len);
Ok(bytes)
}
}
}

impl<Endian, T> EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
/// Construct a new `EndianReader` with the given bytes.
#[inline]
pub fn new(bytes: T, endian: Endian) -> EndianReader<Endian, T> {
let start = 0;
let end = bytes.len();
EndianReader {
range: SubRange { bytes, start, end },
range: SubRange::new(bytes),
endian,
}
}
Expand All @@ -216,7 +243,7 @@ where
impl<Endian, T> EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
/// Take the given `start..end` range of the underlying buffer and return a
/// new `EndianReader`.
Expand All @@ -236,10 +263,8 @@ where
/// Panics if the range is out of bounds.
pub fn range(&self, idx: Range<usize>) -> EndianReader<Endian, T> {
let mut r = self.clone();
r.range.end = r.range.start + idx.end;
r.range.start += idx.start;
assert!(r.range.start <= r.range.end);
assert!(r.range.end <= self.range.end);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is nice to have all this encapsulated.

r.range.skip(idx.start);
r.range.truncate(idx.len());
r
}

Expand All @@ -261,8 +286,7 @@ where
/// Panics if the range is out of bounds.
pub fn range_from(&self, idx: RangeFrom<usize>) -> EndianReader<Endian, T> {
let mut r = self.clone();
r.range.start += idx.start;
assert!(r.range.start <= r.range.end);
r.range.skip(idx.start);
r
}

Expand All @@ -284,27 +308,26 @@ where
/// Panics if the range is out of bounds.
pub fn range_to(&self, idx: RangeTo<usize>) -> EndianReader<Endian, T> {
let mut r = self.clone();
r.range.end = r.range.start + idx.end;
assert!(r.range.end <= self.range.end);
r.range.truncate(idx.end);
r
}
}

impl<Endian, T> Index<usize> for EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
type Output = u8;
fn index(&self, idx: usize) -> &Self::Output {
&self.range.bytes()[idx]
&self.bytes()[idx]
}
}

impl<Endian, T> Index<RangeFrom<usize>> for EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
type Output = [u8];
fn index(&self, idx: RangeFrom<usize>) -> &Self::Output {
Expand All @@ -315,7 +338,7 @@ where
impl<Endian, T> Deref for EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
type Target = [u8];
fn deref(&self) -> &Self::Target {
Expand All @@ -326,7 +349,7 @@ where
impl<Endian, T> Reader for EndianReader<Endian, T>
where
Endian: Endianity,
T: Deref<Target = [u8]> + Clone + Debug,
T: CloneStableDeref<Target = [u8]> + Debug,
{
type Endian = Endian;
type Offset = usize;
Expand All @@ -343,15 +366,15 @@ where

#[inline]
fn empty(&mut self) {
self.range.start = self.range.end;
self.range.truncate(0);
}

#[inline]
fn truncate(&mut self, len: usize) -> Result<()> {
if self.len() < len {
Err(Error::UnexpectedEof)
} else {
self.range.end = self.range.start + len;
self.range.truncate(len);
Ok(())
}
}
Expand All @@ -367,15 +390,15 @@ where

#[inline]
fn find(&self, byte: u8) -> Result<usize> {
self.range.find(byte).ok_or(Error::UnexpectedEof)
self.bytes().iter().position(|x| *x == byte).ok_or(Error::UnexpectedEof)
}

#[inline]
fn skip(&mut self, len: usize) -> Result<()> {
if self.len() < len {
Err(Error::UnexpectedEof)
} else {
self.range.start += len;
self.range.skip(len);
Ok(())
}
}
Expand All @@ -386,8 +409,8 @@ where
Err(Error::UnexpectedEof)
} else {
let mut r = self.clone();
r.range.end = r.range.start + len;
self.range.start += len;
r.range.truncate(len);
self.range.skip(len);
Ok(r)
}
}
Expand Down Expand Up @@ -429,7 +452,7 @@ mod tests {
use endianity::NativeEndian;
use reader::Reader;

fn native_reader<T: Deref<Target = [u8]> + Clone + Debug>(
fn native_reader<T: CloneStableDeref<Target = [u8]> + Debug>(
bytes: T,
) -> EndianReader<NativeEndian, T> {
EndianReader::new(bytes, NativeEndian)
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ extern crate core as std;
extern crate arrayvec;
extern crate byteorder;
extern crate fallible_iterator;
extern crate stable_deref_trait;

#[cfg(feature = "std")]
mod imports {
Expand All @@ -214,6 +215,8 @@ mod imports {

use imports::*;

pub use stable_deref_trait::{CloneStableDeref, StableDeref};

mod cfi;
pub use cfi::*;

Expand Down