Skip to content

Commit 3993707

Browse files
fix: panic when using FastPFOR::new (#53)
Both `page_size` and `block_size` don't make sense to have as zero
1 parent b1794df commit 3993707

4 files changed

Lines changed: 29 additions & 22 deletions

File tree

benches/fastpfor_benchmark.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use core::ops::Range;
2+
use std::hint::black_box;
3+
use std::io::Cursor;
4+
use std::num::NonZeroU32;
5+
26
use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
37
use fastpfor::rust::{FastPFOR, Integer, BLOCK_SIZE_128, BLOCK_SIZE_256, DEFAULT_PAGE_SIZE};
48
use rand::rngs::StdRng;
59
use rand::{RngExt as _, SeedableRng};
6-
use std::hint::black_box;
7-
use std::io::Cursor;
810

911
const SIZES: &[usize; 2] = &[1024, 4096];
1012
const SEED: u64 = 456;
@@ -92,7 +94,7 @@ fn compress_data(codec: &mut FastPFOR, data: &[u32]) -> usize {
9294
}
9395

9496
/// Helper function to compress data and return compressed buffer
95-
fn prepare_compressed_data(data: &[u32], block_size: u32) -> Vec<u32> {
97+
fn prepare_compressed_data(data: &[u32], block_size: NonZeroU32) -> Vec<u32> {
9698
let mut codec = FastPFOR::new(DEFAULT_PAGE_SIZE, block_size);
9799
let mut compressed = vec![0u32; data.len() * 2];
98100
let mut input_offset = Cursor::new(0);

src/rust/integer_compression/fastpfor.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
11
use std::io::Cursor;
2+
use std::num::NonZeroU32;
23

34
use crate::rust::cursor::IncrementCursor;
45
use crate::rust::integer_compression::{bitpacking, helpers};
56
use crate::rust::{FastPForResult, Integer, Skippable};
67
use bytes::{Buf as _, BufMut as _, BytesMut};
78

89
/// Block size constant for 256 integers per block
9-
pub const BLOCK_SIZE_256: u32 = 256;
10+
pub const BLOCK_SIZE_256: NonZeroU32 = NonZeroU32::new(256).unwrap();
1011

1112
/// Block size constant for 128 integers per block
12-
pub const BLOCK_SIZE_128: u32 = 128;
13+
pub const BLOCK_SIZE_128: NonZeroU32 = NonZeroU32::new(128).unwrap();
1314

1415
/// Overhead cost (in bits) for storing each exception's position in the block
1516
const OVERHEAD_OF_EACH_EXCEPT: u32 = 8;
1617

1718
/// Default page size in number of integers
18-
pub const DEFAULT_PAGE_SIZE: u32 = 65536;
19+
pub const DEFAULT_PAGE_SIZE: NonZeroU32 = NonZeroU32::new(65536).unwrap();
1920

2021
/// Fast Patched Frame-of-Reference ([`FastPFOR`](https://github.com/lemire/FastPFor)) integer compression codec.
2122
///
@@ -74,7 +75,7 @@ impl Skippable for FastPFOR {
7475
output_offset: &mut Cursor<u32>,
7576
num: u32,
7677
) -> FastPForResult<()> {
77-
if inlength == 0 && self.block_size == BLOCK_SIZE_128 {
78+
if inlength == 0 && self.block_size == BLOCK_SIZE_128.get() {
7879
// Return early if there is no data to uncompress and block size is 128
7980
return Ok(());
8081
}
@@ -143,7 +144,9 @@ impl FastPFOR {
143144
/// Creates codec with specified page and block sizes.
144145
///
145146
/// Pre-allocates buffers for metadata and exception storage.
146-
pub fn new(page_size: u32, block_size: u32) -> FastPFOR {
147+
pub fn new(page_size: NonZeroU32, block_size: NonZeroU32) -> FastPFOR {
148+
let page_size = page_size.get();
149+
let block_size = block_size.get();
147150
FastPFOR {
148151
page_size,
149152
block_size,
@@ -453,7 +456,7 @@ mod tests {
453456
fn fastpfor_test() {
454457
let mut codec1 = FastPFOR::default();
455458
let mut codec2 = FastPFOR::default();
456-
let mut data = vec![0u32; BLOCK_SIZE_256 as usize];
459+
let mut data = vec![0u32; BLOCK_SIZE_256.get() as usize];
457460
data[126] = -1i32 as u32;
458461
let mut out_buf = vec![0; data.len() * 4];
459462
let mut input_offset = Cursor::new(0);
@@ -483,7 +486,9 @@ mod tests {
483486
.unwrap();
484487
let answer = out_buf_uncomp[..output_offset.position() as usize].to_vec();
485488

486-
for k in 0..BLOCK_SIZE_256 {
489+
assert_eq!(answer.len(), BLOCK_SIZE_256.get() as usize);
490+
assert_eq!(data.len(), BLOCK_SIZE_256.get() as usize);
491+
for k in 0..BLOCK_SIZE_256.get() {
487492
assert_eq!(answer[k as usize], data[k as usize], "bug in {k}");
488493
}
489494
}
@@ -492,10 +497,7 @@ mod tests {
492497
fn fastpfor_test_128() {
493498
let mut codec1 = FastPFOR::new(DEFAULT_PAGE_SIZE, BLOCK_SIZE_128);
494499
let mut codec2 = FastPFOR::new(DEFAULT_PAGE_SIZE, BLOCK_SIZE_128);
495-
let mut data = vec![0; BLOCK_SIZE_128 as usize];
496-
for i in 0..BLOCK_SIZE_128 {
497-
data[i as usize] = 0;
498-
}
500+
let mut data = vec![0; BLOCK_SIZE_128.get() as usize];
499501
data[126] = -1i32 as u32;
500502
let mut out_buf = vec![0; data.len() * 4];
501503
let mut input_offset = Cursor::new(0);
@@ -525,7 +527,9 @@ mod tests {
525527
.unwrap();
526528
let answer = out_buf_uncomp[..output_offset.position() as usize].to_vec();
527529

528-
for k in 0..BLOCK_SIZE_128 {
530+
assert_eq!(answer.len(), BLOCK_SIZE_128.get() as usize);
531+
assert_eq!(data.len(), BLOCK_SIZE_128.get() as usize);
532+
for k in 0..BLOCK_SIZE_128.get() {
529533
assert_eq!(answer[k as usize], data[k as usize], "bug in {k}");
530534
}
531535
}

tests/basic_tests.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(clippy::needless_range_loop)]
33

44
use std::io::Cursor;
5+
use std::num::NonZeroU32;
56

67
use fastpfor::rust::{
78
fast_pack, fast_unpack, Composition, FastPFOR, Integer, VariableByte, BLOCK_SIZE_128,
@@ -381,10 +382,10 @@ fn test_random_numbers() {
381382
#[test]
382383
fn test_fastpfor_headless_compress_unfit_pagesize() {
383384
// The input size is a multiple of 128 but does not fit the page size
384-
let test_input_size = 512 + BLOCK_SIZE_128;
385-
let page_size = 512;
385+
let test_input_size = BLOCK_SIZE_128.checked_add(512).unwrap();
386+
let page_size = NonZeroU32::new(512).unwrap();
386387

387-
let input: Vec<u32> = (0..test_input_size).collect();
388+
let input: Vec<u32> = (0..test_input_size.get()).collect();
388389
let mut output: Vec<u32> = vec![0; input.len()];
389390
let mut decoded: Vec<u32> = vec![0; input.len()];
390391
let mut input_offset = Cursor::new(0u32);
@@ -419,8 +420,8 @@ fn test_fastpfor_headless_compress_unfit_pagesize() {
419420

420421
#[test]
421422
fn test_exception_value_vector_resizes() {
422-
let page_size = 512;
423-
let test_input_size = page_size * 2;
423+
let page_size = NonZeroU32::new(512).unwrap();
424+
let test_input_size = page_size.get() * 2;
424425

425426
// every even index value is large which will trigger exception buffer to be resize
426427
let input: Vec<u32> = (0..test_input_size)

tests/cpp_compat_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fn test_rust_decompresses_cpp_encoded_data() {
1515
let mut codec_rs = rust::FastPFOR::new(rust::DEFAULT_PAGE_SIZE, rust::BLOCK_SIZE_128);
1616

1717
for n in test_input_sizes() {
18-
for input in get_test_cases(n + rust::BLOCK_SIZE_128 as usize) {
18+
for input in get_test_cases(n + rust::BLOCK_SIZE_128.get() as usize) {
1919
// Buffer for the C++ encoded
2020
let mut compressed_buffer = vec![0; input.len()];
2121

@@ -58,7 +58,7 @@ fn test_rust_and_cpp_fastpfor32_compression_matches() {
5858
let mut codec_rs = rust::FastPFOR::new(rust::DEFAULT_PAGE_SIZE, rust::BLOCK_SIZE_128);
5959

6060
for n in test_input_sizes() {
61-
for input in get_test_cases(n + rust::BLOCK_SIZE_128 as usize) {
61+
for input in get_test_cases(n + rust::BLOCK_SIZE_128.get() as usize) {
6262
// Buffer for the C++ encoded
6363
let mut compressed_buffer = vec![0; input.len()];
6464

0 commit comments

Comments
 (0)