fix(dict-builder): stabilize optimized dictionary scoring
Optimized dictionary scoring used the requested dictionary ID while building its scoring CDict, so --dictID could change candidate quality and output content. Parallel FastCover could also choose different equal-score candidates based on completion order. Clear the ID only while constructing the scoring CDict, restore it immediately afterward, and use deterministic (d, k) tie-breakers in Cover and FastCover. The requested ID now remains output metadata instead of influencing dictionary content or repeat-run stability. Test Plan: - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo test --manifest-path rust/Cargo.toml scoring_dictionary_id_is_restored_after_the_score --lib - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings - ulimit -v 41943040; make -j1 - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo test --manifest-path rust/Cargo.toml --all-targets - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo test --manifest-path rust/cli/Cargo.toml --all-targets - ulimit -v 41943040; make -j1 -C tests test
This commit is contained in:
@@ -541,7 +541,10 @@ fn best_finish_slot(
|
|||||||
.lock()
|
.lock()
|
||||||
.unwrap_or_else(|poison| poison.into_inner());
|
.unwrap_or_else(|poison| poison.into_inner());
|
||||||
data.live_jobs = data.live_jobs.wrapping_sub(1);
|
data.live_jobs = data.live_jobs.wrapping_sub(1);
|
||||||
if selection.totalCompressedSize < data.compressed_size {
|
let improves_score = selection.totalCompressedSize < data.compressed_size;
|
||||||
|
let wins_equal_score = selection.totalCompressedSize == data.compressed_size
|
||||||
|
&& (parameters.d, parameters.k) < (data.parameters.d, data.parameters.k);
|
||||||
|
if improves_score || wins_equal_score {
|
||||||
if data.dict.is_null() || data.dict_capacity < selection.dictSize {
|
if data.dict.is_null() || data.dict_capacity < selection.dictSize {
|
||||||
let replacement = unsafe { malloc_bytes(selection.dictSize) };
|
let replacement = unsafe { malloc_bytes(selection.dictSize) };
|
||||||
if replacement.is_null() && selection.dictSize != 0 {
|
if replacement.is_null() && selection.dictSize != 0 {
|
||||||
@@ -629,6 +632,22 @@ unsafe extern "C" {
|
|||||||
) -> usize;
|
) -> usize;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
unsafe fn with_dictionary_id_cleared<T>(
|
||||||
|
dict: *mut u8,
|
||||||
|
dict_buffer_capacity: usize,
|
||||||
|
action: impl FnOnce() -> T,
|
||||||
|
) -> T {
|
||||||
|
if dict_buffer_capacity < 8 {
|
||||||
|
return action();
|
||||||
|
}
|
||||||
|
let dict_id = unsafe { dict.add(4).cast::<u32>() };
|
||||||
|
let saved_id = unsafe { ptr::read_unaligned(dict_id) };
|
||||||
|
unsafe { ptr::write_unaligned(dict_id, 0) };
|
||||||
|
let result = action();
|
||||||
|
unsafe { ptr::write_unaligned(dict_id, saved_id) };
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
#[no_mangle]
|
#[no_mangle]
|
||||||
pub unsafe extern "C" fn COVER_sum(samples_sizes: *const usize, nb_samples: c_uint) -> usize {
|
pub unsafe extern "C" fn COVER_sum(samples_sizes: *const usize, nb_samples: c_uint) -> usize {
|
||||||
if nb_samples == 0 {
|
if nb_samples == 0 {
|
||||||
@@ -781,12 +800,19 @@ pub unsafe extern "C" fn COVER_checkTotalCompressedSize(
|
|||||||
}
|
}
|
||||||
let dst = unsafe { malloc_bytes(dst_capacity) };
|
let dst = unsafe { malloc_bytes(dst_capacity) };
|
||||||
let cctx = unsafe { ZSTD_createCCtx() };
|
let cctx = unsafe { ZSTD_createCCtx() };
|
||||||
|
/* The dictionary ID is output metadata, not dictionary content. The
|
||||||
|
* fixed-frame `ZSTD_compress_usingCDict()` score includes that metadata,
|
||||||
|
* so temporarily clear it while constructing the scoring CDict. This
|
||||||
|
* keeps `--dictID` from changing which candidate wins optimization while
|
||||||
|
* leaving the requested ID in the finalized dictionary. */
|
||||||
let cdict = unsafe {
|
let cdict = unsafe {
|
||||||
ZSTD_createCDict(
|
with_dictionary_id_cleared(dict, dict_buffer_capacity, || {
|
||||||
dict.cast(),
|
ZSTD_createCDict(
|
||||||
dict_buffer_capacity,
|
dict.cast(),
|
||||||
parameters.zParams.compressionLevel,
|
dict_buffer_capacity,
|
||||||
)
|
parameters.zParams.compressionLevel,
|
||||||
|
)
|
||||||
|
})
|
||||||
};
|
};
|
||||||
if dst.is_null() || cctx.is_null() || cdict.is_null() {
|
if dst.is_null() || cctx.is_null() || cdict.is_null() {
|
||||||
if !cctx.is_null() {
|
if !cctx.is_null() {
|
||||||
@@ -1287,6 +1313,22 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoring_dictionary_id_is_restored_after_the_score() {
|
||||||
|
let mut dictionary = [0u8; 16];
|
||||||
|
dictionary[4..8].copy_from_slice(&0x1234_5678u32.to_ne_bytes());
|
||||||
|
let observed_id = unsafe {
|
||||||
|
with_dictionary_id_cleared(dictionary.as_mut_ptr(), dictionary.len(), || {
|
||||||
|
ptr::read_unaligned(dictionary.as_ptr().add(4).cast::<u32>())
|
||||||
|
})
|
||||||
|
};
|
||||||
|
assert_eq!(observed_id, 0);
|
||||||
|
assert_eq!(
|
||||||
|
u32::from_ne_bytes(dictionary[4..8].try_into().unwrap()),
|
||||||
|
0x1234_5678
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn context_groups_repeated_dmers_by_sample() {
|
fn context_groups_repeated_dmers_by_sample() {
|
||||||
let samples = [b'a'; 45];
|
let samples = [b'a'; 45];
|
||||||
|
|||||||
@@ -524,7 +524,10 @@ impl FastBest {
|
|||||||
.lock()
|
.lock()
|
||||||
.unwrap_or_else(|poison| poison.into_inner());
|
.unwrap_or_else(|poison| poison.into_inner());
|
||||||
state.live_jobs = state.live_jobs.wrapping_sub(1);
|
state.live_jobs = state.live_jobs.wrapping_sub(1);
|
||||||
if selection.totalCompressedSize < state.compressed_size {
|
let improves_score = selection.totalCompressedSize < state.compressed_size;
|
||||||
|
let wins_equal_score = selection.totalCompressedSize == state.compressed_size
|
||||||
|
&& (parameters.d, parameters.k) < (state.parameters.d, state.parameters.k);
|
||||||
|
if improves_score || wins_equal_score {
|
||||||
if state.dict.is_null() || state.dict_size < selection.dictSize {
|
if state.dict.is_null() || state.dict_size < selection.dictSize {
|
||||||
unsafe { free_bytes(state.dict) };
|
unsafe { free_bytes(state.dict) };
|
||||||
let replacement = unsafe { malloc_bytes(selection.dictSize) };
|
let replacement = unsafe { malloc_bytes(selection.dictSize) };
|
||||||
|
|||||||
Reference in New Issue
Block a user