Conversation
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Merging this PR will degrade performance by 17.48%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mix[50%_in/50%_out] |
338.9 µs | 391.2 µs | -13.35% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.01)] |
495.1 µs | 582.9 µs | -15.07% |
| ⚡ | Simulation | decompress_rd[f32, (100000, 0.0)] |
583.5 µs | 495.9 µs | +17.65% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.1)] |
495.1 µs | 582.9 µs | -15.07% |
| ⚡ | Simulation | decompress_rd[f64, (10000, 0.01)] |
138.6 µs | 122.3 µs | +13.41% |
| ⚡ | Simulation | decompress_rd[f64, (10000, 0.1)] |
138.7 µs | 122.4 µs | +13.33% |
| ⚡ | Simulation | decompress_rd[f64, (10000, 0.0)] |
138.5 µs | 122.4 µs | +13.17% |
| ❌ | Simulation | decompress_rd[f64, (100000, 0.01)] |
842.6 µs | 1,020.8 µs | -17.46% |
| ❌ | Simulation | decompress_rd[f64, (100000, 0.1)] |
842.5 µs | 1,021 µs | -17.48% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
275.3 ns | 246.1 ns | +11.85% |
Comparing bp/vectt (7cc57ab) with develop (c4feed7)
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
|
|
||
| impl FromArrowArray<&ArrowStructArray> for ArrayRef { | ||
| fn from_arrow(value: &ArrowStructArray, nullable: bool) -> VortexResult<Self> { | ||
| Self::from_arrow_with_session(value, nullable, &LEGACY_SESSION) |
There was a problem hiding this comment.
can we deprecate this method
There was a problem hiding this comment.
Do you mean the ones without session arg? Can do but the surface area is huge, it should probably be a separate mechanical PR
There was a problem hiding this comment.
Yep. could we do a PR which only does that and another (stacked/following) that does this semantic change?
# Conflicts: # vortex-array/public-api.lock
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
| // Temporal extensions stay wrapped — `to_arrow_temporal` reads their metadata. | ||
| // Other extensions unwrap to storage; their identity lives on the Field. | ||
| if let DType::Extension(ext) = self.dtype() | ||
| && ext.metadata_opt::<AnyTemporal>().is_none() | ||
| { | ||
| let ext = self.execute::<ExtensionArray>(ctx)?; | ||
| return ext.storage_array().clone().execute_arrow(data_type, ctx); |
There was a problem hiding this comment.
Does this impl all arrow ext type have the same layout as vortex ones
There was a problem hiding this comment.
Extension is just a wrapper around storage types so yes, they will have the same layout. One difference is there is no extension DataType in arrow, extension types are just Field metadata. We just convert the storage type and attach the right metadata on the field when converting a vortex ext type to arrow.
2 participants
Vortex extension dtypes registered on a session now survive the Arrow boundary in both directions: schema-level (Schema / RecordBatch / nested fields) and leaf-array-level (
pa.ExtensionArrayover the C ABI).