Skip to content

Commit

Permalink
use references to strings in the file where possible instead of alway…
Browse files Browse the repository at this point in the history
…s allocating copies
  • Loading branch information
Ryan-rsm-McKenzie committed Jan 24, 2024
1 parent 3566f89 commit 4a39359
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 163 deletions.
29 changes: 28 additions & 1 deletion src/containers.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use core::ops::Range;
use memmap2::Mmap;
use std::sync::Arc;

Expand Down Expand Up @@ -39,6 +40,12 @@ enum BytesInner<'bytes> {

use BytesInner::*;

impl From<Mapping> for BytesInner<'static> {
fn from(value: Mapping) -> Self {
Mapped(value)
}
}

#[derive(Clone, Debug)]
pub(crate) struct Bytes<'bytes> {
inner: BytesInner<'bytes>,
Expand Down Expand Up @@ -115,6 +122,26 @@ impl<'bytes> Bytes<'bytes> {
},
}
}

#[must_use]
pub(crate) fn copy_slice(&self, slice: Range<usize>) -> Self {
match &self.inner {
Owned(x) => Self {
inner: Owned(x[slice].into()),
},
Borrowed(x) => Self {
inner: Borrowed(&x[slice]),
},
Mapped(x) => Self {
inner: Mapping {
pos: x.pos + slice.start,
len: slice.len(),
mapping: x.mapping.clone(),
}
.into(),
},
}
}
}

impl Bytes<'static> {
Expand All @@ -128,7 +155,7 @@ impl Bytes<'static> {
#[must_use]
pub(crate) fn from_mapped(pos: usize, len: usize, mapping: Arc<Mmap>) -> Self {
Self {
inner: Mapped(Mapping { pos, len, mapping }),
inner: Mapping { pos, len, mapping }.into(),
}
}
}
Expand Down
68 changes: 48 additions & 20 deletions src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,47 +225,72 @@ pub(crate) use compressable_bytes;
macro_rules! key {
($this:ident: $hash:ident) => {
#[derive(::core::clone::Clone, ::core::fmt::Debug, ::core::default::Default)]
pub struct $this {
pub hash: $hash,
pub name: ::bstr::BString,
pub struct $this<'bytes> {
pub(crate) hash: $hash,
pub(crate) name: crate::containers::Bytes<'bytes>,
}

impl<'bytes> $this<'bytes> {
#[must_use]
pub fn hash(&self) -> &$hash {
&self.hash
}

#[must_use]
pub fn name(&self) -> &::bstr::BStr {
::bstr::BStr::new(self.name.as_bytes())
}
}

// false positive
#[allow(clippy::unconditional_recursion)]
impl ::core::cmp::PartialEq for $this {
impl<'bytes> ::core::cmp::PartialEq for $this<'bytes> {
fn eq(&self, other: &Self) -> bool {
self.hash.eq(&other.hash)
}
}

impl ::core::cmp::Eq for $this {}
impl<'bytes> ::core::cmp::Eq for $this<'bytes> {}

impl ::core::cmp::PartialOrd for $this {
impl<'bytes> ::core::cmp::PartialOrd for $this<'bytes> {
fn partial_cmp(&self, other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl ::core::cmp::Ord for $this {
impl<'bytes> ::core::cmp::Ord for $this<'bytes> {
fn cmp(&self, other: &Self) -> ::core::cmp::Ordering {
self.hash.cmp(&other.hash)
}
}

impl ::core::borrow::Borrow<$hash> for $this {
impl<'bytes> ::core::borrow::Borrow<$hash> for $this<'bytes> {
fn borrow(&self) -> &$hash {
&self.hash
}
}

impl<T> ::core::convert::From<T> for $this
impl ::core::convert::From<$hash> for $this<'static> {
fn from(value: $hash) -> Self {
Self {
hash: value,
name: crate::containers::Bytes::default(),
}
}
}

impl<T> ::core::convert::From<T> for $this<'static>
where
T: Into<::bstr::BString>,
T: ::core::convert::Into<::bstr::BString>,
{
fn from(value: T) -> Self {
let mut name = value.into();
let hash = Self::hash_in_place(&mut name);
Self { hash, name }
let v: Vec<u8> = name.into();
Self {
hash,
name: crate::containers::Bytes::from_owned(v.into()),
}
}
}
};
Expand All @@ -275,7 +300,8 @@ pub(crate) use key;

macro_rules! mapping {
($this:ident, $mapping:ident: ($key:ident: $hash:ident) => $value:ident) => {
pub(crate) type $mapping<'bytes> = ::std::collections::BTreeMap<$key, $value<'bytes>>;
pub(crate) type $mapping<'bytes> =
::std::collections::BTreeMap<$key<'bytes>, $value<'bytes>>;

impl<'bytes> crate::Sealed for $this<'bytes> {}

Expand All @@ -301,7 +327,7 @@ macro_rules! mapping {
pub fn get_key_value<K>(
&self,
key: &K,
) -> ::core::option::Option<(&$key, &$value<'bytes>)>
) -> ::core::option::Option<(&$key<'bytes>, &$value<'bytes>)>
where
K: ::core::borrow::Borrow<$hash>,
{
Expand All @@ -322,7 +348,7 @@ macro_rules! mapping {
value: $value<'bytes>,
) -> ::core::option::Option<$value<'bytes>>
where
K: ::core::convert::Into<$key>,
K: ::core::convert::Into<$key<'bytes>>,
{
self.map.insert(key.into(), value)
}
Expand All @@ -332,17 +358,19 @@ macro_rules! mapping {
self.map.is_empty()
}

pub fn iter(&self) -> impl ::core::iter::Iterator<Item = (&$key, &$value<'bytes>)> {
pub fn iter(
&self,
) -> impl ::core::iter::Iterator<Item = (&$key<'bytes>, &$value<'bytes>)> {
self.map.iter()
}

pub fn iter_mut(
&mut self,
) -> impl ::core::iter::Iterator<Item = (&$key, &mut $value<'bytes>)> {
) -> impl ::core::iter::Iterator<Item = (&$key<'bytes>, &mut $value<'bytes>)> {
self.map.iter_mut()
}

pub fn keys(&self) -> impl ::core::iter::Iterator<Item = &$key> {
pub fn keys(&self) -> impl ::core::iter::Iterator<Item = &$key<'bytes>> {
self.map.keys()
}

Expand All @@ -366,7 +394,7 @@ macro_rules! mapping {
pub fn remove_entry<K>(
&mut self,
key: &K,
) -> ::core::option::Option<($key, $value<'bytes>)>
) -> ::core::option::Option<($key<'bytes>, $value<'bytes>)>
where
K: ::core::borrow::Borrow<$hash>,
{
Expand All @@ -384,10 +412,10 @@ macro_rules! mapping {
}
}

impl<'bytes> ::core::iter::FromIterator<($key, $value<'bytes>)> for $this<'bytes> {
impl<'bytes> ::core::iter::FromIterator<($key<'bytes>, $value<'bytes>)> for $this<'bytes> {
fn from_iter<T>(iter: T) -> Self
where
T: ::core::iter::IntoIterator<Item = ($key, $value<'bytes>)>,
T: ::core::iter::IntoIterator<Item = ($key<'bytes>, $value<'bytes>)>,
{
Self {
map: iter.into_iter().collect(),
Expand Down
49 changes: 23 additions & 26 deletions src/fo4/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl Options {

derive::key!(Key: FileHash);

impl Key {
impl<'bytes> Key<'bytes> {
#[must_use]
fn hash_in_place(name: &mut BString) -> FileHash {
fo4::hash_file_in_place(name)
Expand All @@ -174,7 +174,7 @@ impl<'bytes> Archive<'bytes> {
Self::write_header(&mut sink, &header)?;

for (key, file) in self {
Self::write_file(&mut sink, &header, &mut offsets, &key.hash, file)?;
Self::write_file(&mut sink, &header, &mut offsets, key.hash(), file)?;
}

for file in self.values() {
Expand All @@ -185,7 +185,7 @@ impl<'bytes> Archive<'bytes> {

if options.strings {
for key in self.keys() {
sink.write_protocol::<WString>(key.name.as_ref(), Endian::Little)?;
sink.write_protocol::<WString>(key.name(), Endian::Little)?;
}
}

Expand Down Expand Up @@ -400,14 +400,14 @@ impl<'bytes> Archive<'bytes> {
source: &mut In,
header: &Header,
strings: &mut usize,
) -> Result<(Key, File<'bytes>)>
) -> Result<(Key<'bytes>, File<'bytes>)>
where
In: ?Sized + Source<'bytes>,
{
let name = if *strings == 0 {
BString::default()
Bytes::default()
} else {
source.save_restore_position(|source| -> Result<BString> {
source.save_restore_position(|source| -> Result<Bytes<'bytes>> {
source.seek_absolute(*strings)?;
let name = source.read_protocol::<WString>(Endian::Little)?;
*strings = source.stream_position();
Expand Down Expand Up @@ -526,7 +526,7 @@ mod tests {
cc,
fo4::{
Archive, ArchiveKey, ArchiveOptions, ChunkExtra, CompressionFormat, File, FileHeader,
Format, Hash, Version,
Format, Version,
},
prelude::*,
Borrowed,
Expand Down Expand Up @@ -619,16 +619,13 @@ mod tests {
#[test]
fn write_general_archives() -> anyhow::Result<()> {
let root = Path::new("data/fo4_write_test/data");
let make_key =
|file: u32, extension: &[u8], directory: u32, path: &'static str| ArchiveKey {
hash: Hash {
file,
extension: cc::make_four(extension),
directory,
}
.into(),
name: path.into(),
};
let make_key = |file: u32, extension: &[u8], directory: u32, path: &'static str| {
let key: ArchiveKey = path.into();
assert_eq!(key.hash().file, file);
assert_eq!(key.hash().extension, cc::make_four(extension));
assert_eq!(key.hash().directory, directory);
key
};

let keys = [
make_key(
Expand All @@ -643,16 +640,16 @@ mod tests {
0xD9A32978,
"Characters/character_0003.png",
),
make_key(0x36F72750, b"png", 0x60648919, "Construct 3/Readme.txt"),
make_key(0xCA042B67, b"png", 0x29246A47, "Share/License.txt"),
make_key(0x36F72750, b"txt", 0x60648919, "Construct 3/Readme.txt"),
make_key(0xCA042B67, b"txt", 0x29246A47, "Share/License.txt"),
make_key(0xDA3773A6, b"png", 0x0B0A447E, "Tilemap/tiles.png"),
make_key(0x785183FF, b"png", 0xDA3773A6, "Tiles/tile_0003.png"),
];

let mappings: Vec<_> = keys
.iter()
.map(|key| {
let path: PathBuf = [root.as_os_str(), key.name.to_os_str_lossy().as_ref()]
let path: PathBuf = [root.as_os_str(), key.name().to_os_str_lossy().as_ref()]
.into_iter()
.collect();
let fd = fs::File::open(&path)
Expand Down Expand Up @@ -686,19 +683,19 @@ mod tests {
assert_eq!(main.len(), child.len());

for (key, mapping) in keys.iter().zip(&mappings) {
let file = child
.get_key_value(key)
.with_context(|| format!("failed to get file: {}", key.name.to_str_lossy()))?;
assert_eq!(file.0.hash, key.hash);
let file = child.get_key_value(key).with_context(|| {
format!("failed to get file: {}", key.name().to_str_lossy())
})?;
assert_eq!(file.0.hash(), key.hash());
assert_eq!(file.1.len(), 1);
if strings {
assert_eq!(file.0.name, key.name);
assert_eq!(file.0.name(), key.name());
}

let chunk = &file.1[0];
let decompressed_chunk = if chunk.is_compressed() {
let result = chunk.decompress(&Default::default()).with_context(|| {
format!("failed to decompress chunk: {}", key.name.to_str_lossy())
format!("failed to decompress chunk: {}", key.name().to_str_lossy())
})?;
Some(result)
} else {
Expand Down
Loading

0 comments on commit 4a39359

Please sign in to comment.