Skip to content

Commit

Permalink
Allows unwrap in tests and benchmarks + cleanups
Browse files Browse the repository at this point in the history
  • Loading branch information
Artxiom committed May 14, 2024
1 parent 575ed7c commit 90d2e78
Show file tree
Hide file tree
Showing 63 changed files with 204 additions and 110 deletions.
3 changes: 3 additions & 0 deletions crates/re_data_store/benches/arrow2.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! Keeping track of performance issues/regressions in `arrow2` that directly affect us.
// Allow unwrap() in benchmarks
#![allow(clippy::unwrap_used)]

#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/benches/data_store.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Allow unwrap() in benchmarks
#![allow(clippy::unwrap_used)]

#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

Expand Down
22 changes: 9 additions & 13 deletions crates/re_data_store/benches/gc.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Allow unwrap() in benchmarks
#![allow(clippy::unwrap_used)]

#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

Expand All @@ -7,7 +10,6 @@ use itertools::Itertools;
use re_data_store::{
DataStore, DataStoreConfig, GarbageCollectionOptions, GarbageCollectionTarget,
};
use re_log::ResultExt;
use re_log_types::{
build_frame_nr, build_log_time, DataRow, DataTable, EntityPath, RowId, TableId, Time, TimePoint,
};
Expand Down Expand Up @@ -157,9 +159,8 @@ where
for _ in 0..NUM_ROWS_PER_ENTITY_PATH {
#[allow(clippy::needless_range_loop)] // readability
for i in 0..NUM_ENTITY_PATHS {
if let Some(Ok(row)) = rows_per_table[i].next() {
store.insert_row(&row).ok_or_log_error();
}
let row = rows_per_table[i].next().unwrap();
store.insert_row(&row.unwrap()).unwrap();
}
}

Expand All @@ -178,7 +179,7 @@ where
{
let mut table = DataTable::from_rows(
TableId::ZERO,
(0..NUM_ROWS_PER_ENTITY_PATH).filter_map(move |i| {
(0..NUM_ROWS_PER_ENTITY_PATH).map(move |i| {
DataRow::from_component_batches(
RowId::new(),
timegen(i),
Expand All @@ -188,19 +189,14 @@ where
.iter()
.map(|batch| batch as &dyn ComponentBatch),
)
.ok_or_log_error()
.unwrap()
}),
);

// Do a serialization roundtrip to pack everything in contiguous memory.
if packed {
if let Some(t) = table
.serialize()
.and_then(|(schema, columns)| DataTable::deserialize(TableId::ZERO, &schema, &columns))
.ok_or_log_error()
{
table = t;
}
let (schema, columns) = table.serialize().unwrap();
table = DataTable::deserialize(TableId::ZERO, &schema, &columns).unwrap();
}

table.compute_all_size_bytes();
Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/tests/correctness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//!
//! Bending and twisting the datastore APIs in all kinds of weird ways to try and break them.
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use rand::Rng;

use re_data_store::{
Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/tests/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//!
//! Testing & demonstrating expected usage of the datastore APIs, no funny stuff.
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use itertools::Itertools;
use rand::Rng;
use re_data_store::{
Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/tests/dump.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! Dumping a datastore to log messages and back.
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use itertools::Itertools;
use re_data_store::{
test_row,
Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/tests/internals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//!
//! They're awful, but sometimes you just have to…
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use re_data_store::{DataStore, DataStoreConfig};
use re_log_types::{
build_frame_nr, example_components::MyIndex, DataRow, EntityPath, RowId, TimePoint,
Expand Down
3 changes: 3 additions & 0 deletions crates/re_data_store/tests/memory_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! Measures the memory overhead of the data store.
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use std::sync::atomic::{AtomicUsize, Ordering::Relaxed};

thread_local! {
Expand Down
3 changes: 3 additions & 0 deletions crates/re_entity_db/tests/clear.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use re_data_store::{DataStore, LatestAtQuery};
use re_entity_db::EntityDb;
use re_log_types::{
Expand Down
3 changes: 3 additions & 0 deletions crates/re_entity_db/tests/time_histograms.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use std::collections::BTreeSet;

use re_data_store::GarbageCollectionOptions;
Expand Down
3 changes: 3 additions & 0 deletions crates/re_log_encoding/benches/msg_encode_benchmark.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Allow unwrap() in benchmarks
#![allow(clippy::unwrap_used)]

#[cfg(not(all(feature = "decoder", feature = "encoder")))]
compile_error!("msg_encode_benchmark requires 'decoder' and 'encoder' features.");

Expand Down
8 changes: 5 additions & 3 deletions crates/re_log_encoding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,14 @@ impl FileHeader {

#[cfg(feature = "decoder")]
pub fn decode(read: &mut impl std::io::Read) -> Result<Self, decoder::DecodeError> {
let to_array_4b = |slice: &[u8]| slice.try_into().expect("always returns an Ok() variant");

let mut buffer = [0_u8; Self::SIZE];
read.read_exact(&mut buffer)
.map_err(decoder::DecodeError::Read)?;
let magic = buffer[0..4].try_into().unwrap();
let version = buffer[4..8].try_into().unwrap();
let options = EncodingOptions::from_bytes(buffer[8..].try_into().unwrap())?;
let magic = to_array_4b(&buffer[0..4]);
let version = to_array_4b(&buffer[4..8]);
let options = EncodingOptions::from_bytes(to_array_4b(&buffer[8..]))?;
Ok(Self {
magic,
version,
Expand Down
3 changes: 3 additions & 0 deletions crates/re_query/benches/latest_at.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
//! Contains:
//! - A 1:1 port of the benchmarks in `crates/re_query/benches/query_benchmarks.rs`, with caching enabled.
// Allow unwrap() in benchmarks
#![allow(clippy::unwrap_used)]

use criterion::{criterion_group, criterion_main, Criterion};

use itertools::Itertools;
Expand Down
3 changes: 1 addition & 2 deletions crates/re_query/src/latest_at/results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,5 @@ fn downcast<C: Component>(cached: &(dyn ErasedFlatVecDeque + Send + Sync)) -> cr
if cached.num_entries() != 1 {
return Err(anyhow::anyhow!("latest_at deque must be single entry").into());
}
// unwrap checked just above ^^^
Ok(cached.iter().next().unwrap())
Ok(cached.iter().next().expect("checked just above ^^^"))
}
3 changes: 3 additions & 0 deletions crates/re_query/tests/latest_at.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use re_data_store::{DataStore, LatestAtQuery, StoreSubscriber};
use re_log_types::{
build_frame_nr,
Expand Down
3 changes: 3 additions & 0 deletions crates/re_query/tests/range.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// https://github.com/rust-lang/rust-clippy/issues/10011
#![cfg(test)]

use itertools::Itertools as _;

use re_data_store::{DataStore, RangeQuery, ResolvedTimeRange, StoreSubscriber as _, TimeInt};
Expand Down
18 changes: 12 additions & 6 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ impl RecordingStreamBuilder {
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn connect(self) -> RecordingStreamResult<RecordingStream> {
self.connect_opts(crate::default_server_addr(), crate::default_flush_timeout())
self.connect_opts(
crate::default_server_addr(),
Some(crate::default_flush_timeout()),
)
}

/// Creates a new [`RecordingStream`] that is pre-configured to stream the data through to a
Expand All @@ -325,7 +328,7 @@ impl RecordingStreamBuilder {
///
/// ```no_run
/// let rec = re_sdk::RecordingStreamBuilder::new("rerun_example_app")
/// .connect_opts(re_sdk::default_server_addr(), re_sdk::default_flush_timeout())?;
/// .connect_opts(re_sdk::default_server_addr(), Some(re_sdk::default_flush_timeout()))?;
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn connect_opts(
Expand Down Expand Up @@ -422,7 +425,7 @@ impl RecordingStreamBuilder {
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn spawn(self) -> RecordingStreamResult<RecordingStream> {
self.spawn_opts(&Default::default(), crate::default_flush_timeout())
self.spawn_opts(&Default::default(), Some(crate::default_flush_timeout()))
}

/// Spawns a new Rerun Viewer process from an executable available in PATH, then creates a new
Expand All @@ -442,7 +445,7 @@ impl RecordingStreamBuilder {
///
/// ```no_run
/// let rec = re_sdk::RecordingStreamBuilder::new("rerun_example_app")
/// .spawn_opts(&re_sdk::SpawnOptions::default(), re_sdk::default_flush_timeout())?;
/// .spawn_opts(&re_sdk::SpawnOptions::default(), Some(re_sdk::default_flush_timeout()))?;
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn spawn_opts(
Expand Down Expand Up @@ -1510,7 +1513,10 @@ impl RecordingStream {
/// terms of data durability and ordering.
/// See [`Self::set_sink`] for more information.
pub fn connect(&self) {
self.connect_opts(crate::default_server_addr(), crate::default_flush_timeout());
self.connect_opts(
crate::default_server_addr(),
Some(crate::default_flush_timeout()),
);
}

/// Swaps the underlying sink for a [`crate::log_sink::TcpSink`] sink pre-configured to use
Expand Down Expand Up @@ -1552,7 +1558,7 @@ impl RecordingStream {
/// terms of data durability and ordering.
/// See [`Self::set_sink`] for more information.
pub fn spawn(&self) -> RecordingStreamResult<()> {
self.spawn_opts(&Default::default(), crate::default_flush_timeout())
self.spawn_opts(&Default::default(), Some(crate::default_flush_timeout()))
}

/// Spawns a new Rerun Viewer process from an executable available in PATH, then swaps the
Expand Down
5 changes: 2 additions & 3 deletions crates/re_sdk_comms/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ pub fn default_server_addr() -> std::net::SocketAddr {
}

/// The default amount of time to wait for the TCP connection to resume during a flush
#[allow(clippy::unnecessary_wraps)]
pub fn default_flush_timeout() -> Option<std::time::Duration> {
Some(std::time::Duration::from_secs(2))
pub fn default_flush_timeout() -> std::time::Duration {
std::time::Duration::from_secs(2)
}
6 changes: 5 additions & 1 deletion crates/re_sdk_comms/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,11 @@ impl CongestionManager {
// decision on a time we've previously seen,
// thus sending parts of a sequence-time instead of all-or-nothing.
while timeline_history.send_time.len() > 1024 {
let oldest_time = *timeline_history.send_time.keys().next().unwrap();
let oldest_time = *timeline_history
.send_time
.keys()
.next()
.expect("safe because checked above");
timeline_history.send_time.remove(&oldest_time);
}

Expand Down
5 changes: 3 additions & 2 deletions crates/re_types/src/components/half_sizes2d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/half_sizes3d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/line_strip2d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/line_strip3d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/pinhole_projection.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/position2d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/position3d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/range1d.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/resolution.rs

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

5 changes: 3 additions & 2 deletions crates/re_types/src/components/texcoord2d.rs

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

Loading

0 comments on commit 90d2e78

Please sign in to comment.