Skip to content

Commit 5d867f7

Browse files
Roderick Boveeboydgreenfield
authored andcommitted
Fix bugs in feature-gated rank and expose more mmap guts
1 parent 7588157 commit 5d867f7

File tree

3 files changed

+23
-13
lines changed

3 files changed

+23
-13
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ murmurhash3 = "0.0.5"
99

1010
[features]
1111
backward_bytes = []
12+
rank_lookup = []
1213

1314
[dev-dependencies]
1415
bencher = "0.1.5"

src/combinatorial.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,29 @@ pub fn rank(value: usize, k: u8) -> u128 {
1414
// `RUST_TEST_THREADS=1 cargo test` or else you'll get tons of
1515
// errors because of data races between threads with different k's
1616

17-
// also some tests (i.e. ones with k=1) fail because there are fewer
18-
// than 200000 possible values for the table
1917
unsafe {
2018
// clear out the lookup table if we have a new k and fill
2119
// it with values for the new k
2220
if MARKER_BITS != k {
21+
let mut table_size = MARKER_TABLE_SIZE;
22+
if k == 1 {
23+
table_size = 128;
24+
} else if k == 2 {
25+
table_size = 8128;
26+
}
2327
MARKER_TABLE = [0; MARKER_TABLE_SIZE];
24-
MARKER_TABLE[0] = (1 << k) - 1 as BitVecSlice;
25-
for i in 1..MARKER_TABLE_SIZE {
26-
MARKER_TABLE[i] = next_marker(MARKER_TABLE[i - 1]);
28+
MARKER_TABLE[0] = (1 << k) - 1;
29+
for i in 1..table_size {
30+
MARKER_TABLE[i] = next_rank(MARKER_TABLE[i - 1]);
2731
}
2832
MARKER_BITS = k;
2933
}
34+
// it's possible this may overflow if value > (128 choose k) or return
35+
// a bad value (0) if value > (128 choose k) and k == 1 or 2
3036
if value as usize >= MARKER_TABLE_SIZE {
3137
let mut marker = MARKER_TABLE[MARKER_TABLE_SIZE - 1];
3238
for _ in 0..(value as usize - MARKER_TABLE_SIZE) {
33-
marker = next_marker(marker);
39+
marker = next_rank(marker);
3440
}
3541
marker
3642
} else {
@@ -156,7 +162,7 @@ pub fn choose(n: u64, k: u8) -> u64 {
156162
}
157163
}
158164
TryFrom::try_from(num / denom)
159-
.expect(&format!("{} choose {} is greater than 2**64", n, k))
165+
.unwrap_or_else(|_| panic!("{} choose {} is greater than 2**64", n, k))
160166
// (or recursively) choose(n - 1, k - 1) + choose(n-1, k)
161167
// for floats, this should work since they handle fractions:
162168
// (1..u64::from(k)).map(|i| (n + 1 - i) / i).product(),

src/mmap_bitvec.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,22 @@ use bitvec::BitVector;
1111

1212
/// Really annoying we have to mock over both of these rather than memmap
1313
/// just providing support.
14-
enum CommonMmap {
14+
pub enum CommonMmap {
1515
MmapMut(MmapMut),
1616
Mmap(Mmap),
1717
}
1818

1919
impl CommonMmap {
2020
#[inline]
21-
fn as_ptr(&self) -> *const u8 {
21+
pub fn as_ptr(&self) -> *const u8 {
2222
match self {
2323
CommonMmap::MmapMut(x) => x.as_ptr(),
2424
CommonMmap::Mmap(x) => x.as_ptr(),
2525
}
2626
}
2727

2828
#[inline]
29-
fn as_mut_ptr(&mut self) -> *mut u8 {
29+
pub fn as_mut_ptr(&mut self) -> *mut u8 {
3030
// maybe we should override the original type signature and return
3131
// a Result<*mut u8, io::Error> here with the read-only failure?
3232
match self {
@@ -36,7 +36,7 @@ impl CommonMmap {
3636
}
3737

3838
#[inline]
39-
fn flush(&mut self) -> Result<(), io::Error> {
39+
pub fn flush(&mut self) -> Result<(), io::Error> {
4040
match self {
4141
CommonMmap::MmapMut(x) => x.flush(),
4242
CommonMmap::Mmap(_) => Ok(()),
@@ -56,7 +56,7 @@ impl CommonMmap {
5656
/// assert_eq!(bv.get_range(2..12), 0b1001101101);
5757
/// ```
5858
pub struct MmapBitVec {
59-
mmap: CommonMmap,
59+
pub mmap: CommonMmap,
6060
pub size: usize,
6161
header: Box<[u8]>,
6262
}
@@ -122,7 +122,10 @@ impl MmapBitVec {
122122
// we have to open with write=true to satisfy MmapMut (which we're
123123
// using because there's no generic over both MmapMut and Mmap so
124124
// picking one simplifies the types)
125-
let mut file = OpenOptions::new().read(true).write(!read_only).open(filename)?;
125+
let mut file = OpenOptions::new()
126+
.read(true)
127+
.write(!read_only)
128+
.open(filename)?;
126129

127130
// read the magic bytes and (optionally) check if it matches
128131
let mut file_magic = [0; 2];

0 commit comments

Comments
 (0)