Skip to content

Commit 4aa1c15

Browse files
authored
perf: Arc::ptr_eq short-circuit to DType eq comparisons (#7398)
FieldName, FieldNames, and StructFields all wrap Arc types but their derived PartialEq always dereferences through the Arc to compare contents. Since DTypes are frequently cloned (every slice, filter, execute copies the DType), the cloned Arcs share pointers - making pointer equality a reliable fast path. DataFusion ClickBench full-suite [apmc](https://github.com/0ax1/apmc) measurement for vortex, averaged over two runs: - Cycles: -5.7% (861B → 812B) — fewer total CPU cycles - IPC: +5.8% (1.88 → 1.99) — more instructions per cycle - MAP_STALL: -5.5% (360B → 340B) — less CPU stalls waiting on memory Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
1 parent 3bdb376 commit 4aa1c15

3 files changed

Lines changed: 27 additions & 12 deletions

File tree

vortex-array/public-api.lock

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9758,7 +9758,7 @@ pub fn vortex_array::dtype::FieldName::cmp(&self, other: &vortex_array::dtype::F
97589758

97599759
impl core::cmp::PartialEq for vortex_array::dtype::FieldName
97609760

9761-
pub fn vortex_array::dtype::FieldName::eq(&self, other: &vortex_array::dtype::FieldName) -> bool
9761+
pub fn vortex_array::dtype::FieldName::eq(&self, other: &Self) -> bool
97629762

97639763
impl core::cmp::PartialEq<&alloc::string::String> for vortex_array::dtype::FieldName
97649764

@@ -9832,8 +9832,6 @@ impl core::hash::Hash for vortex_array::dtype::FieldName
98329832

98339833
pub fn vortex_array::dtype::FieldName::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
98349834

9835-
impl core::marker::StructuralPartialEq for vortex_array::dtype::FieldName
9836-
98379835
pub struct vortex_array::dtype::FieldNames(_)
98389836

98399837
impl vortex_array::dtype::FieldNames
@@ -9858,7 +9856,7 @@ impl core::cmp::Eq for vortex_array::dtype::FieldNames
98589856

98599857
impl core::cmp::PartialEq for vortex_array::dtype::FieldNames
98609858

9861-
pub fn vortex_array::dtype::FieldNames::eq(&self, other: &vortex_array::dtype::FieldNames) -> bool
9859+
pub fn vortex_array::dtype::FieldNames::eq(&self, other: &Self) -> bool
98629860

98639861
impl core::cmp::PartialEq<&[&str]> for &vortex_array::dtype::FieldNames
98649862

@@ -9928,8 +9926,6 @@ pub type vortex_array::dtype::FieldNames::Item = vortex_array::dtype::FieldName
99289926

99299927
pub fn vortex_array::dtype::FieldNames::into_iter(self) -> Self::IntoIter
99309928

9931-
impl core::marker::StructuralPartialEq for vortex_array::dtype::FieldNames
9932-
99339929
impl core::ops::index::Index<usize> for vortex_array::dtype::FieldNames
99349930

99359931
pub type vortex_array::dtype::FieldNames::Output = vortex_array::dtype::FieldName
@@ -10170,7 +10166,7 @@ impl core::cmp::Eq for vortex_array::dtype::StructFields
1017010166

1017110167
impl core::cmp::PartialEq for vortex_array::dtype::StructFields
1017210168

10173-
pub fn vortex_array::dtype::StructFields::eq(&self, other: &vortex_array::dtype::StructFields) -> bool
10169+
pub fn vortex_array::dtype::StructFields::eq(&self, other: &Self) -> bool
1017410170

1017510171
impl core::default::Default for vortex_array::dtype::StructFields
1017610172

@@ -10188,8 +10184,6 @@ impl core::hash::Hash for vortex_array::dtype::StructFields
1018810184

1018910185
pub fn vortex_array::dtype::StructFields::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
1019010186

10191-
impl core::marker::StructuralPartialEq for vortex_array::dtype::StructFields
10192-
1019310187
impl vortex_array::dtype::arrow::FromArrowType<&arrow_schema::fields::Fields> for vortex_array::dtype::StructFields
1019410188

1019510189
pub fn vortex_array::dtype::StructFields::from_arrow(value: &arrow_schema::fields::Fields) -> Self

vortex-array/src/dtype/field_names.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,16 @@ use itertools::Itertools;
1010
use vortex_utils::aliases::StringEscape;
1111

1212
/// A name for a field in a struct.
13-
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
13+
#[derive(Clone, Debug, Eq, PartialOrd, Ord, Hash)]
14+
#[allow(clippy::derived_hash_with_manual_eq)] // manual PartialEq adds Arc::ptr_eq fast path only
1415
pub struct FieldName(Arc<str>);
1516

17+
impl PartialEq for FieldName {
18+
fn eq(&self, other: &Self) -> bool {
19+
Arc::ptr_eq(&self.0, &other.0) || *self.0 == *other.0
20+
}
21+
}
22+
1623
impl FieldName {
1724
/// Returns a reference to the inner string
1825
pub fn inner(&self) -> &Arc<str> {
@@ -139,10 +146,17 @@ impl From<FieldName> for Arc<str> {
139146
}
140147

141148
/// An ordered list of field names in a struct.
142-
#[derive(Clone, PartialEq, Eq, Debug, Default, Hash)]
149+
#[derive(Clone, Eq, Debug, Default, Hash)]
150+
#[allow(clippy::derived_hash_with_manual_eq)] // manual PartialEq adds Arc::ptr_eq fast path only
143151
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
144152
pub struct FieldNames(Arc<[FieldName]>);
145153

154+
impl PartialEq for FieldNames {
155+
fn eq(&self, other: &Self) -> bool {
156+
Arc::ptr_eq(&self.0, &other.0) || *self.0 == *other.0
157+
}
158+
}
159+
146160
impl fmt::Display for FieldNames {
147161
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
148162
write!(

vortex-array/src/dtype/struct_.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,16 @@ impl FieldDTypeInner {
138138
/// // Accessing a field by name will yield the first
139139
/// assert_eq!(fields.field("int_col").unwrap(), DType::Primitive(PType::I32, Nullability::Nullable));
140140
/// ```
141-
#[derive(Clone, PartialEq, Eq, Hash)]
141+
#[derive(Clone, Eq, Hash)]
142+
#[allow(clippy::derived_hash_with_manual_eq)] // manual PartialEq adds Arc::ptr_eq fast path only
142143
pub struct StructFields(Arc<StructFieldsInner>);
143144

145+
impl PartialEq for StructFields {
146+
fn eq(&self, other: &Self) -> bool {
147+
Arc::ptr_eq(&self.0, &other.0) || self.0 == other.0
148+
}
149+
}
150+
144151
impl std::fmt::Debug for StructFields {
145152
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
146153
f.debug_struct("StructFields")

0 commit comments

Comments
 (0)