Skip to content

Commit

Permalink
Merge pull request #63 from luftkode/62-integer-overflow-for-large-da…
Browse files Browse the repository at this point in the history
…tasets

62 integer overflow for large datasets
  • Loading branch information
CramBL authored Oct 9, 2024
2 parents 3238833 + 2577d51 commit 22a3360
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 25 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

## [0.18.3]

### Fix

- Integer overflow when searching for the appropriate downsampled MipMap level.

## [0.18.2]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "logviewer-rs"
description = "Log viewer app for viewing plots of data from projects such as motor and generator control"
authors = ["SkyTEM Surveys", "Marc Beck König"]
version = "0.18.2"
version = "0.18.3"
edition = "2021"
repository = "https://github.com/luftkode/logviewer-rs"
homepage = "https://github.com/luftkode/logviewer-rs"
Expand Down
52 changes: 29 additions & 23 deletions crates/plot_util/src/mipmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
// Avoid repeated calls to num_levels()
let num_levels = self.num_levels();

let x_max_adjusted = x_max.max(1); // Precompute x_max.max(1) to avoid repeated calls

// If not found in cache, compute it
for lvl_idx in (0..num_levels).rev() {
let lvl = &self.data[lvl_idx];
Expand All @@ -214,28 +212,9 @@ impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
continue;
}

// Binary search optimization: find approximate positions first
let approx_start = (lvl.len() * x_min) / x_max_adjusted;
let approx_end = (lvl.len() * x_max) / x_max_adjusted;

let approx_start_min = approx_start.min(lvl.len());
let approx_end_min = approx_end.min(lvl.len());

// Narrow search ranges using approximations
let start_idx = if approx_start > 0 {
let start_search = &lvl[..approx_start_min];
start_search.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_min)
+ approx_start_min
} else {
lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_min)
};

let end_idx = if approx_end < lvl.len() {
let end_search = &lvl[approx_end_min..];
end_search.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_max) + approx_end_min
} else {
lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_max)
};
let start_idx = lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_min);
let end_idx = lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_max);

// Use saturating_sub for safety and to avoid potential panic
let count_within_bounds = end_idx.saturating_sub(start_idx);
Expand Down Expand Up @@ -281,6 +260,11 @@ mod tests {
use super::*;
use pretty_assertions::assert_eq;

#[cfg(not(target_pointer_width = "32"))]
const UNIX_TS_NS: usize = 1_728_470_564_000_000_000;
#[cfg(target_pointer_width = "32")]
const UNIX_TS_NS: usize = 1_728_470_564;

#[test]
fn test_mipmap_strategy_max() {
let source: Vec<[f64; 2]> = vec![[1.1, 2.2], [3.3, 4.4], [5.5, 1.1], [7.7, 3.3]];
Expand Down Expand Up @@ -381,6 +365,28 @@ mod tests {
}
}

/// Test for: `<https://github.com/luftkode/logviewer-rs/issues/62>`
#[test]
fn test_level_match_large_timestamps() {
let source_len = 1600;
let source: Vec<[f64; 2]> = (0..source_len)
.map(|i| [(i + UNIX_TS_NS) as f64, (i + UNIX_TS_NS) as f64])
.collect();
let mut mipmap = MipMap2D::new(source, MipMapStrategy::Min);

let x_bounds = (UNIX_TS_NS + 300, UNIX_TS_NS + 5000);

for (pixel_width, expected_lvl) in
[(100usize, 3usize), (200, 2), (400, 1), (800, 0), (1600, 0)]
{
assert_eq!(
mipmap.get_level_match(pixel_width, x_bounds),
expected_lvl,
"Expected lvl {expected_lvl} for width: {pixel_width}"
);
}
}

#[test]
fn test_out_of_bounds_level() {
let source: Vec<[f64; 2]> = vec![[1.0, 2.0], [3.0, 4.0]];
Expand Down
12 changes: 12 additions & 0 deletions crates/skytem_logs/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,18 @@ mod tests {
let log = GeneratorLog::from_reader(&mut data.as_slice())?;

let first_entry = log.entries().first().expect("Empty entries");

let first_ts_ns = first_entry.timestamp_ns();
assert_eq!(
first_ts_ns,
NaiveDate::from_ymd_opt(2023, 1, 24)
.expect("Invalid arguments")
.and_hms_opt(13, 47, 45)
.expect("Invalid arguments")
.and_utc()
.timestamp_nanos_opt()
.expect("timestamp as nanoseconds out of range") as f64
);
assert_eq!(
first_entry.timestamp,
NaiveDate::from_ymd_opt(2023, 1, 24)
Expand Down

0 comments on commit 22a3360

Please sign in to comment.