Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all public types are Send + Sync #227

Merged
merged 3 commits into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"workflow_dispatch": {},
"pull_request": {
"branches": [
"main"
"main",
"develop-*"
],
"types": [
"opened",
Expand Down
5 changes: 5 additions & 0 deletions serde_arrow/src/internal/array_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,8 @@ impl std::convert::AsMut<ArrayBuilder> for ArrayBuilder {
self
}
}

const _: () = {
trait AssertSendSync: Send + Sync {}
impl AssertSendSync for ArrayBuilder {}
};
5 changes: 5 additions & 0 deletions serde_arrow/src/internal/deserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,8 @@ impl<'de> serde::de::Deserializer<'de> for Deserializer<'de> {
false
}
}

const _: () = {
trait AssertSendSync: Send + Sync {}
impl<'de> AssertSendSync for Deserializer<'de> {}
};
6 changes: 6 additions & 0 deletions serde_arrow/src/internal/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,9 @@ impl<E: std::fmt::Display> From<E> for PanicOnErrorError {
panic!("{value}");
}
}

const _: () = {
trait AssertSendSync: Send + Sync {}
impl AssertSendSync for Error {}
impl<T: Send + Sync> AssertSendSync for Result<T> {}
};
7 changes: 7 additions & 0 deletions serde_arrow/src/internal/schema/extensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ mod variable_shape_tensor_field;
pub use bool8_field::Bool8Field;
pub use fixed_shape_tensor_field::FixedShapeTensorField;
pub use variable_shape_tensor_field::VariableShapeTensorField;

const _: () = {
trait AssertSendSync: Send + Sync {}
impl AssertSendSync for Bool8Field {}
impl AssertSendSync for FixedShapeTensorField {}
impl AssertSendSync for VariableShapeTensorField {}
};
8 changes: 8 additions & 0 deletions serde_arrow/src/internal/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,11 @@ impl<'a> std::fmt::Display for DataTypeDisplay<'a> {
}
}
}

const _: () = {
trait AssertSendSync: Send + Sync {}
impl AssertSendSync for SerdeArrowSchema {}
impl AssertSendSync for TracingOptions {}
impl AssertSendSync for Strategy {}
impl AssertSendSync for Overwrites {}
};
18 changes: 14 additions & 4 deletions serde_arrow/src/internal/serialization/struct_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,23 @@ impl SimpleSerializer for StructBuilder {
}
}

/// Optimize field lookups for static names
#[derive(Debug, Clone)]
pub struct FieldLookup {
pub cached_names: Vec<Option<(*const u8, usize)>>,
pub cached_names: Vec<Option<StaticFieldName>>,
pub index: BTreeMap<String, usize>,
}

/// A wrapper around a static field name that compares using ptr and length
#[derive(Debug, Clone)]
pub struct StaticFieldName(&'static str);

impl std::cmp::PartialEq for StaticFieldName {
fn eq(&self, other: &Self) -> bool {
(self.0.as_ptr(), self.0.len()) == (other.0.as_ptr(), other.0.len())
}
}

impl FieldLookup {
pub fn new(field_names: Vec<String>) -> Result<Self> {
let mut index = BTreeMap::new();
Expand All @@ -234,13 +245,12 @@ impl FieldLookup {
}

pub fn lookup(&mut self, guess: usize, key: &'static str) -> Option<usize> {
let fast_key = (key.as_ptr(), key.len());
if self.cached_names.get(guess) == Some(&Some(fast_key)) {
if self.cached_names.get(guess) == Some(&Some(StaticFieldName(key))) {
Some(guess)
} else {
let &idx = self.index.get(key)?;
if self.cached_names[idx].is_none() {
self.cached_names[idx] = Some(fast_key);
self.cached_names[idx] = Some(StaticFieldName(key));
}
Some(idx)
}
Expand Down
5 changes: 5 additions & 0 deletions serde_arrow/src/internal/serializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,8 @@ impl<A: AsMut<ArrayBuilder>> serde::ser::SerializeTupleVariant for CollectionSer
Ok(Serializer(self.0))
}
}

const _: () = {
trait AssertSendSync: Send + Sync {}
impl<A: Send + Sync> AssertSendSync for Serializer<A> {}
};
2 changes: 1 addition & 1 deletion x.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"on": {
"workflow_dispatch": {},
"pull_request": {
"branches": ["main"],
"branches": ["main", "develop-*"],
"types": [
"opened",
"edited",
Expand Down