-
Notifications
You must be signed in to change notification settings - Fork 0
Further optimization #6
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
Changes from 14 commits
a57a504
c6236a1
5142a3d
1f964de
3627f85
1ebdc7b
a93e21b
d8d77f1
2d7323e
19e4fe3
ce93d55
0314bc7
834e3fd
c4b047d
2211802
dc080a9
1c627e0
146611d
e8a96e6
b36bdd0
d1ae9b8
db7ed70
1a94603
674511a
e944c6f
7a41ac9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,203 @@ | ||||||||||||||||||||||||||||||||
| diff --git a/benches/backend_comparison.rs b/benches/backend_comparison.rs | ||||||||||||||||||||||||||||||||
| index bb9c6be..41d9750 100644 | ||||||||||||||||||||||||||||||||
| --- a/benches/backend_comparison.rs | ||||||||||||||||||||||||||||||||
| +++ b/benches/backend_comparison.rs | ||||||||||||||||||||||||||||||||
| @@ -16,21 +16,22 @@ compile_error!("backend_comparison bench requires `xz`, `xz-sys`, or `liblzma-sy | ||||||||||||||||||||||||||||||||
| #[cfg(feature = "liblzma-sys")] | ||||||||||||||||||||||||||||||||
| use liblzma_sys::{ | ||||||||||||||||||||||||||||||||
| lzma_crc32, lzma_crc64, lzma_easy_buffer_encode, lzma_stream_buffer_bound, | ||||||||||||||||||||||||||||||||
| - lzma_stream_buffer_decode, LZMA_CHECK_CRC64, | ||||||||||||||||||||||||||||||||
| + lzma_stream_buffer_decode, LZMA_CHECK_CRC64, LZMA_OK, | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| #[cfg(feature = "xz")] | ||||||||||||||||||||||||||||||||
| -use xz::check::{crc32_fast::lzma_crc32, crc64_fast::lzma_crc64}; | ||||||||||||||||||||||||||||||||
| -#[cfg(feature = "xz")] | ||||||||||||||||||||||||||||||||
| -use xz::common::{ | ||||||||||||||||||||||||||||||||
| - easy_buffer_encoder::lzma_easy_buffer_encode, stream_buffer_decoder::lzma_stream_buffer_decode, | ||||||||||||||||||||||||||||||||
| - stream_buffer_encoder::lzma_stream_buffer_bound, | ||||||||||||||||||||||||||||||||
| +use xz::{ | ||||||||||||||||||||||||||||||||
| + check::{crc32_fast::lzma_crc32, crc64_fast::lzma_crc64}, | ||||||||||||||||||||||||||||||||
| + common::{ | ||||||||||||||||||||||||||||||||
| + easy_buffer_encoder::lzma_easy_buffer_encode, | ||||||||||||||||||||||||||||||||
| + stream_buffer_decoder::lzma_stream_buffer_decode, | ||||||||||||||||||||||||||||||||
| + stream_buffer_encoder::lzma_stream_buffer_bound, | ||||||||||||||||||||||||||||||||
| + }, | ||||||||||||||||||||||||||||||||
| + types::{LZMA_CHECK_CRC64, LZMA_OK}, | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| -#[cfg(feature = "xz")] | ||||||||||||||||||||||||||||||||
| -use xz::types::LZMA_CHECK_CRC64; | ||||||||||||||||||||||||||||||||
| #[cfg(feature = "xz-sys")] | ||||||||||||||||||||||||||||||||
| use xz_sys::{ | ||||||||||||||||||||||||||||||||
| lzma_crc32, lzma_crc64, lzma_easy_buffer_encode, lzma_stream_buffer_bound, | ||||||||||||||||||||||||||||||||
| - lzma_stream_buffer_decode, LZMA_CHECK_CRC64, | ||||||||||||||||||||||||||||||||
| + lzma_stream_buffer_decode, LZMA_CHECK_CRC64, LZMA_OK, | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| #[cfg(feature = "xz")] | ||||||||||||||||||||||||||||||||
| @@ -56,7 +57,7 @@ unsafe fn backend_encode(input: &[u8]) -> Vec<u8> { | ||||||||||||||||||||||||||||||||
| let bound = lzma_stream_buffer_bound(input.len()); | ||||||||||||||||||||||||||||||||
| let mut out = vec![0u8; bound]; | ||||||||||||||||||||||||||||||||
| let mut out_pos: usize = 0; | ||||||||||||||||||||||||||||||||
| - lzma_easy_buffer_encode( | ||||||||||||||||||||||||||||||||
| + let ret = lzma_easy_buffer_encode( | ||||||||||||||||||||||||||||||||
| 6, | ||||||||||||||||||||||||||||||||
| LZMA_CHECK_CRC64, | ||||||||||||||||||||||||||||||||
| ptr::null(), | ||||||||||||||||||||||||||||||||
| @@ -66,6 +67,7 @@ unsafe fn backend_encode(input: &[u8]) -> Vec<u8> { | ||||||||||||||||||||||||||||||||
| &mut out_pos, | ||||||||||||||||||||||||||||||||
| out.len(), | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| + assert_eq!(ret, LZMA_OK, "{BACKEND_NAME} encode failed with {ret}"); | ||||||||||||||||||||||||||||||||
| out.truncate(out_pos); | ||||||||||||||||||||||||||||||||
| out | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| @@ -75,7 +77,7 @@ unsafe fn backend_decode(compressed: &[u8], out_size: usize) -> Vec<u8> { | ||||||||||||||||||||||||||||||||
| let mut memlimit = u64::MAX; | ||||||||||||||||||||||||||||||||
| let mut in_pos = 0usize; | ||||||||||||||||||||||||||||||||
| let mut out_pos = 0usize; | ||||||||||||||||||||||||||||||||
| - lzma_stream_buffer_decode( | ||||||||||||||||||||||||||||||||
| + let ret = lzma_stream_buffer_decode( | ||||||||||||||||||||||||||||||||
| &mut memlimit, | ||||||||||||||||||||||||||||||||
| 0, | ||||||||||||||||||||||||||||||||
| ptr::null(), | ||||||||||||||||||||||||||||||||
| @@ -86,6 +88,17 @@ unsafe fn backend_decode(compressed: &[u8], out_size: usize) -> Vec<u8> { | ||||||||||||||||||||||||||||||||
| &mut out_pos, | ||||||||||||||||||||||||||||||||
| out.len(), | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| + assert_eq!(ret, LZMA_OK, "{BACKEND_NAME} decode failed with {ret}"); | ||||||||||||||||||||||||||||||||
| + assert_eq!( | ||||||||||||||||||||||||||||||||
| + in_pos, | ||||||||||||||||||||||||||||||||
| + compressed.len(), | ||||||||||||||||||||||||||||||||
| + "{BACKEND_NAME} decode left trailing input: consumed {in_pos} of {} bytes", | ||||||||||||||||||||||||||||||||
| + compressed.len() | ||||||||||||||||||||||||||||||||
| + ); | ||||||||||||||||||||||||||||||||
| + assert_eq!( | ||||||||||||||||||||||||||||||||
| + out_pos, out_size, | ||||||||||||||||||||||||||||||||
| + "{BACKEND_NAME} decode produced {out_pos} bytes, expected {out_size}" | ||||||||||||||||||||||||||||||||
| + ); | ||||||||||||||||||||||||||||||||
| out.truncate(out_pos); | ||||||||||||||||||||||||||||||||
| out | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| diff --git a/perf-probe/src/main.rs b/perf-probe/src/main.rs | ||||||||||||||||||||||||||||||||
| index 37cd579..3a56de6 100644 | ||||||||||||||||||||||||||||||||
| --- a/perf-probe/src/main.rs | ||||||||||||||||||||||||||||||||
| +++ b/perf-probe/src/main.rs | ||||||||||||||||||||||||||||||||
| @@ -225,7 +225,7 @@ fn usage() -> String { | ||||||||||||||||||||||||||||||||
| let mut message = String::new(); | ||||||||||||||||||||||||||||||||
| message.push_str("Usage:\n"); | ||||||||||||||||||||||||||||||||
| message.push_str( | ||||||||||||||||||||||||||||||||
| - " cargo run -p perf-probe --release --no-default-features --features <liblzma-sys|xz-sys> -- \\\n", | ||||||||||||||||||||||||||||||||
| + " cargo run -p perf-probe --release --no-default-features --features <xz|xz-sys|liblzma-sys> -- \\\n", | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| message.push_str(" --workload <encode|decode|size|crc32|crc64> [options]\n\n"); | ||||||||||||||||||||||||||||||||
| message.push_str("Options:\n"); | ||||||||||||||||||||||||||||||||
| @@ -527,6 +527,16 @@ unsafe fn backend_decode(compressed: &[u8], out_size: usize) -> Vec<u8> { | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| assert_eq!(ret, LZMA_OK, "{BACKEND_NAME} decode failed with {ret}"); | ||||||||||||||||||||||||||||||||
| + assert_eq!( | ||||||||||||||||||||||||||||||||
| + in_pos, | ||||||||||||||||||||||||||||||||
| + compressed.len(), | ||||||||||||||||||||||||||||||||
| + "{BACKEND_NAME} decode left trailing input: consumed {in_pos} of {} bytes", | ||||||||||||||||||||||||||||||||
| + compressed.len() | ||||||||||||||||||||||||||||||||
| + ); | ||||||||||||||||||||||||||||||||
| + assert_eq!( | ||||||||||||||||||||||||||||||||
| + out_pos, out_size, | ||||||||||||||||||||||||||||||||
| + "{BACKEND_NAME} decode produced {out_pos} bytes, expected {out_size}" | ||||||||||||||||||||||||||||||||
| + ); | ||||||||||||||||||||||||||||||||
| out.truncate(out_pos); | ||||||||||||||||||||||||||||||||
| out | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| diff --git a/scripts/compare_api_workloads.sh b/scripts/compare_api_workloads.sh | ||||||||||||||||||||||||||||||||
| index 014f70c..15f0b9a 100755 | ||||||||||||||||||||||||||||||||
| --- a/scripts/compare_api_workloads.sh | ||||||||||||||||||||||||||||||||
| +++ b/scripts/compare_api_workloads.sh | ||||||||||||||||||||||||||||||||
| @@ -50,7 +50,7 @@ esac | ||||||||||||||||||||||||||||||||
| RUST_BIN="$RUST_TARGET/release/examples/$EXAMPLE_NAME" | ||||||||||||||||||||||||||||||||
| C_BIN="$C_TARGET/release/examples/$EXAMPLE_NAME" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| -env CARGO_TARGET_DIR="$RUST_TARGET" cargo build --example "$EXAMPLE_NAME" --release >/dev/null | ||||||||||||||||||||||||||||||||
| +env CARGO_TARGET_DIR="$RUST_TARGET" cargo build --example "$EXAMPLE_NAME" --release --no-default-features --features xz >/dev/null | ||||||||||||||||||||||||||||||||
| env LZMA_API_STATIC=1 CARGO_TARGET_DIR="$C_TARGET" cargo build --example "$EXAMPLE_NAME" --release --no-default-features --features liblzma-sys >/dev/null | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| hyperfine \ | ||||||||||||||||||||||||||||||||
| diff --git a/scripts/compare_backends.sh b/scripts/compare_backends.sh | ||||||||||||||||||||||||||||||||
| index fb5628a..f16fb02 100755 | ||||||||||||||||||||||||||||||||
| --- a/scripts/compare_backends.sh | ||||||||||||||||||||||||||||||||
| +++ b/scripts/compare_backends.sh | ||||||||||||||||||||||||||||||||
| @@ -70,7 +70,7 @@ SYSTEST_RUST_CMD="env CARGO_TARGET_DIR=$SYSTEST_RUST_TARGET cargo test -p systes | ||||||||||||||||||||||||||||||||
| SYSTEST_C_CMD="env LZMA_API_STATIC=1 CARGO_TARGET_DIR=$SYSTEST_C_TARGET cargo test -p systest --release --no-default-features --features liblzma-sys -- --test-threads=1" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| echo "prebuilding root test binaries..." | ||||||||||||||||||||||||||||||||
| -env CARGO_TARGET_DIR="$ROOT_RUST_TARGET" cargo test --release --no-run >/dev/null | ||||||||||||||||||||||||||||||||
| +env CARGO_TARGET_DIR="$ROOT_RUST_TARGET" cargo test -p liblzma --release --no-default-features --features xz --no-run >/dev/null | ||||||||||||||||||||||||||||||||
| env LZMA_API_STATIC=1 CARGO_TARGET_DIR="$ROOT_C_TARGET" cargo test --release --no-default-features --features liblzma-sys --no-run >/dev/null | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| hyperfine \ | ||||||||||||||||||||||||||||||||
| diff --git a/scripts/profile_backend.sh b/scripts/profile_backend.sh | ||||||||||||||||||||||||||||||||
| index ee74f22..5bcab7e 100755 | ||||||||||||||||||||||||||||||||
| --- a/scripts/profile_backend.sh | ||||||||||||||||||||||||||||||||
| +++ b/scripts/profile_backend.sh | ||||||||||||||||||||||||||||||||
| @@ -3,11 +3,10 @@ set -euo pipefail | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if [[ $# -lt 2 ]]; then | ||||||||||||||||||||||||||||||||
| cat <<'EOF' >&2 | ||||||||||||||||||||||||||||||||
| -Usage: scripts/profile_backend.sh <c|liblzma-sys|xz|rust|xz-sys|both> <encode|decode|size|crc32|crc64> [backend_probe args...] | ||||||||||||||||||||||||||||||||
| +Usage: scripts/profile_backend.sh <c|liblzma-sys|xz|rust|xz-sys> <encode|decode|size|crc32|crc64> [backend_probe args...] | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Examples: | ||||||||||||||||||||||||||||||||
| scripts/profile_backend.sh xz decode --size 1048576 --iters 500 --warmup 50 | ||||||||||||||||||||||||||||||||
| - scripts/profile_backend.sh both encode --input-kind random --size 8388608 | ||||||||||||||||||||||||||||||||
| scripts/profile_backend.sh xz size --input-kind random --size 1048576 --iters 800 --warmup 80 | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Environment: | ||||||||||||||||||||||||||||||||
| @@ -40,10 +39,6 @@ case "$BACKEND" in | ||||||||||||||||||||||||||||||||
| TARGET_DIR="target/profile-bench-xz-sys" | ||||||||||||||||||||||||||||||||
| BACKEND_ENV=() | ||||||||||||||||||||||||||||||||
| ;; | ||||||||||||||||||||||||||||||||
| - both) | ||||||||||||||||||||||||||||||||
| - echo "profile_backend.sh profiles one backend at a time; use xz, xz-sys, or liblzma-sys" >&2 | ||||||||||||||||||||||||||||||||
| - exit 2 | ||||||||||||||||||||||||||||||||
| - ;; | ||||||||||||||||||||||||||||||||
| *) | ||||||||||||||||||||||||||||||||
| echo "unknown backend: $BACKEND" >&2 | ||||||||||||||||||||||||||||||||
| exit 2 | ||||||||||||||||||||||||||||||||
| diff --git a/xz/src/common/common.rs b/xz/src/common/common.rs | ||||||||||||||||||||||||||||||||
| index ca6108f..b37486d 100644 | ||||||||||||||||||||||||||||||||
| --- a/xz/src/common/common.rs | ||||||||||||||||||||||||||||||||
| +++ b/xz/src/common/common.rs | ||||||||||||||||||||||||||||||||
| @@ -120,6 +120,12 @@ pub unsafe fn lzma_bufcpy( | ||||||||||||||||||||||||||||||||
| out_pos: *mut size_t, | ||||||||||||||||||||||||||||||||
| out_size: size_t, | ||||||||||||||||||||||||||||||||
| ) -> size_t { | ||||||||||||||||||||||||||||||||
| + if *in_pos > in_size || *out_pos > out_size { | ||||||||||||||||||||||||||||||||
| + return 0; | ||||||||||||||||||||||||||||||||
| + } | ||||||||||||||||||||||||||||||||
| + if (in_0.is_null() && *in_pos != in_size) || (out.is_null() && *out_pos != out_size) { | ||||||||||||||||||||||||||||||||
| + return 0; | ||||||||||||||||||||||||||||||||
| + } | ||||||||||||||||||||||||||||||||
| debug_assert!(!in_0.is_null() || *in_pos == in_size); | ||||||||||||||||||||||||||||||||
| debug_assert!(!out.is_null() || *out_pos == out_size); | ||||||||||||||||||||||||||||||||
| debug_assert!(*in_pos <= in_size); | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Runtime guards make subsequent The new early returns cover the exact same conditions as the ♻️ Proposed cleanup if *in_pos > in_size || *out_pos > out_size {
return 0;
}
if (in_0.is_null() && *in_pos != in_size) || (out.is_null() && *out_pos != out_size) {
return 0;
}
- debug_assert!(!in_0.is_null() || *in_pos == in_size);
- debug_assert!(!out.is_null() || *out_pos == out_size);
- debug_assert!(*in_pos <= in_size);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| diff --git a/xz/src/common/stream_decoder.rs b/xz/src/common/stream_decoder.rs | ||||||||||||||||||||||||||||||||
| index 70071fc..791d0c3 100644 | ||||||||||||||||||||||||||||||||
| --- a/xz/src/common/stream_decoder.rs | ||||||||||||||||||||||||||||||||
| +++ b/xz/src/common/stream_decoder.rs | ||||||||||||||||||||||||||||||||
| @@ -173,7 +173,7 @@ unsafe extern "C" fn stream_decode( | ||||||||||||||||||||||||||||||||
| return LZMA_OK; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| (*coder).pos = 0; | ||||||||||||||||||||||||||||||||
| - let mut footer_flags = MaybeUninit::<lzma_stream_flags>::uninit(); | ||||||||||||||||||||||||||||||||
| + let mut footer_flags = MaybeUninit::<lzma_stream_flags>::zeroed(); | ||||||||||||||||||||||||||||||||
| let ret_3: lzma_ret = lzma_stream_footer_decode( | ||||||||||||||||||||||||||||||||
| footer_flags.as_mut_ptr(), | ||||||||||||||||||||||||||||||||
| ::core::ptr::addr_of_mut!((*coder).buffer) as *mut u8, | ||||||||||||||||||||||||||||||||
| @@ -244,6 +244,8 @@ unsafe extern "C" fn stream_decode( | ||||||||||||||||||||||||||||||||
| ::core::ptr::addr_of_mut!((*coder).buffer) as *mut u8, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| if ret_ != LZMA_OK { | ||||||||||||||||||||||||||||||||
| + lzma_filters_free(filters_ptr, allocator); | ||||||||||||||||||||||||||||||||
| + (*coder).block_options.filters = core::ptr::null_mut(); | ||||||||||||||||||||||||||||||||
| return ret_; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| (*coder).block_options.ignore_check = (*coder).ignore_check as lzma_bool; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In Cargo, when the same crate appears in[dependencies]and[dev-dependencies], are features unified for benches/examples/tests, and do default features from the dev-dependency stay enabled?💡 Result:
Yes, when the same crate appears in both [dependencies] and [dev-dependencies], Cargo unifies the features for builds that include dev-dependencies (benches, examples, tests). This uses the union of features specified in both sections. Default features from the dev-dependency declaration remain enabled unless explicitly disabled there (via default-features = false). For normal builds (cargo build on lib/bin), only [dependencies] features apply (dev-deps ignored). This behavior depends on the feature resolver version (default "2" for edition 2021+, controlled by top-level resolver field). Resolver v2 avoids unwanted unification but includes dev-dep features when building dev targets like tests/examples/benches. Example from docs: [dependencies] serde = {version = "1.0", default-features = false} [dev-dependencies] serde = {version = "1.0", features = ["std"]} - cargo build: serde without features (defaults off). - cargo test: serde with "std" (union, defaults from dev-dep enabled since not disabled).
Citations:
🏁 Script executed:
Repository: youknowone/xz-rs
Length of output: 1680
liblzma-sysstill enablesbindgenfor dev targets.Line 45 defines a target-specific dev-dependency
liblzma-sys = { path = "liblzma-sys" }withoutdefault-features = false. When benches/examples/tests are built with theliblzma-sysfeature, Cargo unifies features across dependencies and dev-dependencies, re-enabling the defaultbindgenfeature from the dev-dependency. This defeats the goal of disabling it at Line 25 and keeps the libclang requirement around for those builds.♻️ Proposed fix
[target.'cfg(not(target_family = "wasm"))'.dev-dependencies] xz-sys = { path = "xz-sys" } -liblzma-sys = { path = "liblzma-sys" } +liblzma-sys = { path = "liblzma-sys", default-features = false }🤖 Prompt for AI Agents