Skip to content

Commit

Permalink
Revert "smallvec -> tinyvec (keep the commit around, but need MaybeUn…
Browse files Browse the repository at this point in the history
…init to be viable)"

This reverts commit 10521ea.
  • Loading branch information
teh-cmc committed Apr 3, 2023
1 parent 10521ea commit 843244f
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 50 deletions.
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 crates/re_arrow_store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ indent = "0.1"
itertools = { workspace = true }
nohash-hasher = "0.2"
parking_lot.workspace = true
smallvec = { version = "1.0", features = ["const_generics", "union"] }
static_assertions = "1.1"
thiserror.workspace = true
tinyvec = { version = "1.6", features = ["alloc", "rustc_1_55"] }

# Native dependencies:
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
Expand Down
4 changes: 2 additions & 2 deletions crates/re_arrow_store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::sync::atomic::AtomicU64;
use ahash::HashMap;
use arrow2::array::Int64Array;
use arrow2::datatypes::{DataType, TimeUnit};
use tinyvec::TinyVec;
use smallvec::SmallVec;

use nohash_hasher::{IntMap, IntSet};
use parking_lot::RwLock;
Expand Down Expand Up @@ -62,7 +62,7 @@ impl DataStoreConfig {

// ---

pub type InsertIdVec = TinyVec<[u64; 4]>;
pub type InsertIdVec = SmallVec<[u64; 4]>;

/// Keeps track of datatype information for all component types that have been written to the store
/// so far.
Expand Down
8 changes: 4 additions & 4 deletions crates/re_arrow_store/src/store_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use re_log::trace;
use re_log_types::{
ComponentName, DataCell, EntityPath, MsgId, RowId, TimeInt, TimePoint, TimeRange, Timeline,
};
use tinyvec::TinyVec;
use smallvec::SmallVec;

use crate::{DataStore, IndexedBucket, IndexedBucketInner, IndexedTable, PersistentIndexedTable};

Expand Down Expand Up @@ -899,8 +899,8 @@ impl IndexedBucketInner {

{
crate::profile_scope!("control");
fn reshuffle_control_column<T: Default + Copy, const N: usize>(
column: &mut TinyVec<[T; N]>,
fn reshuffle_control_column<T: Copy, const N: usize>(
column: &mut SmallVec<[T; N]>,
swaps: &[(usize, usize)],
) {
let source = {
Expand Down Expand Up @@ -933,7 +933,7 @@ impl IndexedBucketInner {
{
crate::profile_scope!("rotate");
for (from, to) in swaps.iter().copied() {
column[to] = source[from].take(); // TODO: why take?
column[to] = source[from].take();
}
}
}
Expand Down
29 changes: 21 additions & 8 deletions crates/re_arrow_store/src/store_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use arrow2::datatypes::DataType;
use itertools::Itertools as _;
use nohash_hasher::{IntMap, IntSet};
use parking_lot::RwLock;
use tinyvec::TinyVec;
use smallvec::SmallVec;

use re_log::{debug, trace};
use re_log_types::{
Expand Down Expand Up @@ -486,6 +486,19 @@ impl IndexedBucket {
// Used in debug builds to assert that we've left everything in a sane state.
let _total_rows = col_time1.len();

fn split_off_column<T: Copy, const N: usize>(
column: &mut SmallVec<[T; N]>,
split_idx: usize,
) -> SmallVec<[T; N]> {
if split_idx >= column.len() {
return SmallVec::default();
}

let second_half = SmallVec::from_slice(&column[split_idx..]);
column.truncate(split_idx);
second_half
}

let (min2, bucket2) = {
let split_idx = find_split_index(col_time1).expect("must be splittable at this point");

Expand All @@ -495,13 +508,13 @@ impl IndexedBucket {
// this updates `time_range1` in-place!
split_time_range_off(split_idx, col_time1, time_range1),
// this updates `col_time1` in-place!
col_time1.split_off(split_idx),
split_off_column(col_time1, split_idx),
// this updates `col_insert_id1` in-place!
(!col_insert_id1.is_empty()).then(|| col_insert_id1.split_off(split_idx)),
split_off_column(col_insert_id1, split_idx),
// this updates `col_row_id1` in-place!
col_row_id1.split_off(split_idx),
split_off_column(col_row_id1, split_idx),
// this updates `col_num_instances1` in-place!
col_num_instances1.split_off(split_idx),
split_off_column(col_num_instances1, split_idx),
)
};

Expand All @@ -512,12 +525,12 @@ impl IndexedBucket {
.iter_mut()
.map(|(name, column1)| {
if split_idx >= column1.len() {
return (*name, DataCellColumn(TinyVec::default()));
return (*name, DataCellColumn(SmallVec::default()));
}

// this updates `column1` in-place!
let column2 = DataCellColumn({
let second_half = TinyVec::from(&column1.0[split_idx..]);
let second_half = SmallVec::from(&column1.0[split_idx..]);
column1.0.truncate(split_idx);
second_half
});
Expand All @@ -533,7 +546,7 @@ impl IndexedBucket {
is_sorted: true,
time_range: time_range2,
col_time: col_time2,
col_insert_id: col_insert_id2.unwrap_or_default(),
col_insert_id: col_insert_id2,
col_row_id: col_row_id2,
col_num_instances: col_num_instances2,
columns: columns2,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_log_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ ndarray.workspace = true
nohash-hasher = "0.2"
num-derive = "0.3"
num-traits = "0.2"
smallvec = { version = "1.10", features = ["union"] }
thiserror.workspace = true
time = { workspace = true, default-features = false, features = [
"formatting",
"macros",
] }
tinyvec = { version = "1.6", features = ["alloc", "rustc_1_55"] }
typenum = "1.15"
uuid = { version = "1.1", features = ["serde", "v4", "js"] }

Expand Down
6 changes: 0 additions & 6 deletions crates/re_log_types/src/component_types/msg_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ pub type TableId = MsgId;

pub type RowId = MsgId;

impl Default for MsgId {
fn default() -> Self {
Self::ZERO
}
}

impl std::fmt::Debug for MsgId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{:032X}", self.0.as_u128())
Expand Down
9 changes: 0 additions & 9 deletions crates/re_log_types/src/data_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,6 @@ pub struct DataCell {
pub(crate) values: Box<dyn arrow2::array::Array>,
}

impl Default for DataCell {
fn default() -> Self {
Self {
name: "".into(),
values: arrow2::array::NullArray::new_empty(arrow2::datatypes::DataType::Null).boxed(),
}
}
}

// TODO(cmc): We should be able to build a cell from non-reference types.
// TODO(#1619): We shouldn't have to specify the component name separately, this should be
// part of the metadata by using an extension.
Expand Down
4 changes: 2 additions & 2 deletions crates/re_log_types/src/data_row.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ahash::HashSetExt;
use nohash_hasher::IntSet;
use tinyvec::TinyVec;
use smallvec::SmallVec;

use crate::{ComponentName, DataCell, DataCellError, DataTable, EntityPath, RowId, TimePoint};

Expand Down Expand Up @@ -44,7 +44,7 @@ pub type DataRowResult<T> = ::std::result::Result<T, DataRowError>;

// ---

pub type DataCellVec = TinyVec<[DataCell; 4]>;
type DataCellVec = SmallVec<[DataCell; 4]>;

/// A row's worth of [`DataCell`]s: a collection of independent [`DataCell`]s with different
/// underlying datatypes and pointing to different parts of the heap.
Expand Down
18 changes: 9 additions & 9 deletions crates/re_log_types/src/data_table.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ahash::HashMap;
use itertools::Itertools as _;
use nohash_hasher::{IntMap, IntSet};
use tinyvec::TinyVec;
use smallvec::SmallVec;

use crate::{
ArrowMsg, ComponentName, DataCell, DataCellError, DataRow, DataRowError, EntityPath, RowId,
Expand Down Expand Up @@ -41,17 +41,17 @@ pub type DataTableResult<T> = ::std::result::Result<T, DataTableError>;

// ---

pub type RowIdVec = TinyVec<[RowId; 4]>;
pub type RowIdVec = SmallVec<[RowId; 4]>;

pub type TimePointVec = TinyVec<[TimePoint; 4]>;
pub type TimePointVec = SmallVec<[TimePoint; 4]>;

pub type ErasedTimeVec = TinyVec<[i64; 4]>;
pub type ErasedTimeVec = SmallVec<[i64; 4]>;

pub type EntityPathVec = TinyVec<[EntityPath; 4]>;
pub type EntityPathVec = SmallVec<[EntityPath; 4]>;

pub type NumInstancesVec = TinyVec<[u32; 4]>;
pub type NumInstancesVec = SmallVec<[u32; 4]>;

pub type DataCellOptVec = TinyVec<[Option<DataCell>; 4]>;
pub type DataCellOptVec = SmallVec<[Option<DataCell>; 4]>;

/// A column's worth of [`DataCell`]s: a sparse collection of [`DataCell`]s that share the same
/// underlying type and likely point to shared, contiguous memory.
Expand Down Expand Up @@ -97,7 +97,7 @@ impl std::ops::IndexMut<usize> for DataCellColumn {
impl DataCellColumn {
#[inline]
pub fn empty(num_rows: usize) -> Self {
Self(TinyVec::Heap(vec![None; num_rows]))
Self(smallvec::smallvec![None; num_rows])
}
}

Expand Down Expand Up @@ -317,7 +317,7 @@ impl DataTable {
for component in components {
columns.insert(
component,
DataCellColumn(TinyVec::Heap(vec![None; column.len()])),
DataCellColumn(smallvec::smallvec![None; column.len()]),
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/re_log_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub use self::component_types::{EncodedMesh3D, Mesh3D, MeshFormat, MeshId, RawMe
pub use self::component_types::{MsgId, RowId, TableId};
pub use self::data::*;
pub use self::data_cell::{DataCell, DataCellError, DataCellResult};
pub use self::data_row::{DataCellVec, DataRow, DataRowError, DataRowResult};
pub use self::data_row::{DataRow, DataRowError, DataRowResult};
pub use self::data_table::{
DataCellColumn, DataCellOptVec, DataTable, DataTableError, DataTableResult, EntityPathVec,
ErasedTimeVec, NumInstancesVec, RowIdVec, TimePointVec, COLUMN_ENTITY_PATH,
Expand Down
6 changes: 0 additions & 6 deletions crates/re_log_types/src/path/entity_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ pub struct EntityPath {
path: Arc<EntityPathImpl>,
}

impl Default for EntityPath {
fn default() -> Self {
Self::root()
}
}

impl EntityPath {
#[inline]
pub fn root() -> Self {
Expand Down

0 comments on commit 843244f

Please sign in to comment.