Skip to content

Commit

Permalink
try tree-traversal without thread_local!
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Jun 20, 2023
1 parent b9eb407 commit 5a9a7a3
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 55 deletions.
46 changes: 17 additions & 29 deletions gix-pack/src/cache/delta/traverse/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ where
E: std::error::Error + Send + Sync + 'static,
{
let mut decompressed_bytes_by_pack_offset = BTreeMap::new();
let decompress_from_resolver = |slice: EntryRange, out: &mut Vec<u8>| -> Result<(data::Entry, u64), Error> {
let mut inflate = zlib::Inflate::default();
let mut decompress_from_resolver = |slice: EntryRange, out: &mut Vec<u8>| -> Result<(data::Entry, u64), Error> {
let bytes = resolve(slice.clone(), resolve_data).ok_or(Error::ResolveFailed {
pack_offset: slice.start,
})?;
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len);
let compressed = &bytes[entry.header_size()..];
let decompressed_len = entry.decompressed_size as usize;
decompress_all_at_once_with(compressed, decompressed_len, out)?;
decompress_all_at_once_with(&mut inflate, compressed, decompressed_len, out)?;
Ok((entry, slice.end))
};

Expand Down Expand Up @@ -235,15 +236,16 @@ where
move || -> Result<(), Error> {
let mut fully_resolved_delta_bytes = Vec::new();
let mut delta_bytes = Vec::new();
let decompress_from_resolver =
let mut inflate = zlib::Inflate::default();
let mut decompress_from_resolver =
|slice: EntryRange, out: &mut Vec<u8>| -> Result<(data::Entry, u64), Error> {
let bytes = resolve(slice.clone(), resolve_data).ok_or(Error::ResolveFailed {
pack_offset: slice.start,
})?;
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len);
let compressed = &bytes[entry.header_size()..];
let decompressed_len = entry.decompressed_size as usize;
decompress_all_at_once_with(compressed, decompressed_len, out)?;
decompress_all_at_once_with(&mut inflate, compressed, decompressed_len, out)?;
Ok((entry, slice.end))
};

Expand Down Expand Up @@ -407,31 +409,17 @@ fn set_len(v: &mut Vec<u8>, new_len: usize) {
}
}

fn decompress_all_at_once_with(b: &[u8], decompressed_len: usize, out: &mut Vec<u8>) -> Result<(), Error> {
fn decompress_all_at_once_with(
inflate: &mut zlib::Inflate,
b: &[u8],
decompressed_len: usize,
out: &mut Vec<u8>,
) -> Result<(), Error> {
set_len(out, decompressed_len);
// TODO: try to put this back after the next zlib-ng upgrade.
// This is from 3a2d5286084597d4c68549903709cda77dda4357 and it worked until zlib-ng-sys 1.1.9. Then it started to
// fail with `incorrect data check` 25% of the time.
// Note that thread_local! usage was also removed in two other places in `decode/entry.rs` for good measure.
// use std::cell::RefCell;
// thread_local! {
// pub static INFLATE: RefCell<zlib::Inflate> = RefCell::new(zlib::Inflate::default());
// }
//
// INFLATE.with(|inflate| {
// let mut inflate = inflate.borrow_mut();
// let res = inflate.once(b, out).map_err(|err| Error::ZlibInflate {
// source: err,
// message: "Failed to decompress entry",
// });
// inflate.reset();
// res
// })?;
zlib::Inflate::default()
.once(b, out)
.map_err(|err| Error::ZlibInflate {
source: err,
message: "Failed to decompress entry",
})?;
inflate.reset();
inflate.once(b, out).map_err(|err| Error::ZlibInflate {
source: err,
message: "Failed to decompress entry",
})?;
Ok(())
}
26 changes: 0 additions & 26 deletions gix-pack/src/data/file/decode/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,6 @@ impl File {
let offset: usize = data_offset.try_into().expect("offset representable by machine");
assert!(offset < self.data.len(), "entry offset out of bounds");

// use std::cell::RefCell;
// thread_local! {
// pub static INFLATE: RefCell<zlib::Inflate> = RefCell::new(zlib::Inflate::default());
// }
// INFLATE.with(|inflate| {
// let mut inflate = inflate.borrow_mut();
// let res = inflate
// .once(&self.data[offset..], out)
// .map(|(_status, consumed_in, _consumed_out)| consumed_in);
// inflate.reset();
// res
// })
zlib::Inflate::default()
.once(&self.data[offset..], out)
.map(|(_status, consumed_in, _consumed_out)| consumed_in)
Expand All @@ -152,20 +140,6 @@ impl File {
let offset: usize = data_offset.try_into().expect("offset representable by machine");
assert!(offset < self.data.len(), "entry offset out of bounds");

// TODO: put this back in once we know that zlib-ng doesn't fail once in a million times (see tree-traversal)
// use std::cell::RefCell;
// thread_local! {
// pub static INFLATE: RefCell<zlib::Inflate> = RefCell::new(zlib::Inflate::default());
// }
//
// INFLATE.with(|inflate| {
// let mut inflate = inflate.borrow_mut();
// let res = inflate
// .once(&self.data[offset..], out)
// .map(|(_status, consumed_in, consumed_out)| (consumed_in, consumed_out));
// inflate.reset();
// res
// })
zlib::Inflate::default()
.once(&self.data[offset..], out)
.map(|(_status, consumed_in, consumed_out)| (consumed_in, consumed_out))
Expand Down

0 comments on commit 5a9a7a3

Please sign in to comment.