This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 9a95c6da17 perf(arrow-array): skip `DataType` drop glue in generic
temporal converters (#11896)
9a95c6da17 is described below
commit 9a95c6da1765905c2b64fe782ddeb8e737b571d9
Author: Matt Butrovich <[email protected]>
AuthorDate: Sun Oct 25 08:54:56 2026 +0000
perf(arrow-array): skip `DataType` drop glue in generic temporal converters
(#11896)
# Which issue does this PR close?
- Closes #11895.
# Rationale for this change
The issue describes the problem.
# What changes are included in this PR?
`as_datetime`, `as_datetime_with_timezone`, `as_time` and `as_duration`
wrap the `T::DATA_TYPE` temporary in `ManuallyDrop`, so no drop glue
runs per value. `as_duration` wasn't in the issue, but it has the same
per-value drop.
# Are these changes tested?
Existing tests cover the converters through
`PrimitiveArray::value_as_datetime`, `value_as_time` and the
`arrow-cast` timestamp casts. Behavior is unchanged.
Timestamp casts with a time zone call `as_datetime_with_timezone::<T>`
per value. In the bot's `cast_timestamp` runs, casts to `Date32` got
1.04x to 1.26x faster on both machines. Casts to `Time32`/`Time64` got
up to 1.10x faster on Neoverse-V2 and didn't change on Xeon.
<img width="1600" height="1508" alt="as-datetime-drop-bot"
src="https://github.com/user-attachments/assets/2960905c-1920-4188-9dc7-55499b1ee461";
/>
<details><summary>All runs</summary>
These are speedups over `main` for all 20 time zone cases and the 20
cases without a time zone, which don't call the changed code. The
aligned runs add `-C llvm-args=-align-all-functions=6 -C
llvm-args=-align-all-nofallthru-blocks=6`.
| Machine | Run | To `Date32`, time zone | To `Time32`/`Time64`, time
zone | No time zone |
|---|---|---|---|---|
| Neoverse-V2 |
[default](https://github.com/apache/arrow-rs/pull/11896#issuecomment-6385769963)
| 1.11x to 1.26x | 1.01x to 1.10x | 0.99x to 1.03x |
| Neoverse-V2 |
[aligned](https://github.com/apache/arrow-rs/pull/11896#issuecomment-6385770216)
| 1.12x to 1.25x | 1.02x to 1.08x | 0.99x to 1.00x |
| Xeon 8581C |
[default](https://github.com/apache/arrow-rs/pull/11896#issuecomment-6385769196)
| 1.04x to 1.22x | 0.95x to 1.02x | 0.98x to 1.02x |
| Xeon 8581C |
[aligned](https://github.com/apache/arrow-rs/pull/11896#issuecomment-6385768460)
| 1.06x to 1.13x | 0.96x to 1.03x | 0.97x to 1.00x |
On Apple M5 Max with thin LTO, local runs gave 1.10x to 1.36x for
`Date32` and 1.08x to 1.19x for `Time32`/`Time64`. `format_temporal`
didn't change (0.97x to 1.00x), because formatting costs far more per
value than the call this removes. I haven't profiled why
`Time32`/`Time64` doesn't gain on Xeon.
In the thin-LTO `cast_timestamp` binary on aarch64,
`drop_glue::<DataType>` is no longer called from any `timestamp_to_time`
or `adjust_timestamp_to_timezone` monomorphization. On x86_64, I checked
a `unary_opt` kernel over `as_datetime::<TimestampNanosecondType>`
before linking. It now compiles to the same 1166 instructions as the
same kernel over `timestamp_ns_to_datetime`.
</details>
# Are there any significant user-facing changes?
No.
## AI Disclosure
I wrote this PR with help from Claude but understand and support the
core approach, implementation, and test coverage.
---
arrow-array/src/temporal_conversions.rs | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/arrow-array/src/temporal_conversions.rs
b/arrow-array/src/temporal_conversions.rs
index 0197acdc51..e264a15bdb 100644
--- a/arrow-array/src/temporal_conversions.rs
+++ b/arrow-array/src/temporal_conversions.rs
@@ -21,6 +21,7 @@ use crate::ArrowPrimitiveType;
use crate::timezone::Tz;
use arrow_schema::{DataType, TimeUnit};
use chrono::{DateTime, Duration, NaiveDate, NaiveDateTime, NaiveTime,
TimeZone, Timelike, Utc};
+use std::mem::ManuallyDrop;
/// Number of seconds in a day
pub const SECONDS_IN_DAY: i64 = 86_400;
@@ -241,7 +242,10 @@ pub fn duration_ns_to_duration(v: i64) -> Duration {
/// Converts an [`ArrowPrimitiveType`] to [`NaiveDateTime`]
pub fn as_datetime<T: ArrowPrimitiveType>(v: i64) -> Option<NaiveDateTime> {
- as_datetime_with_data_type(&T::DATA_TYPE, v)
+ // Dropping the `DATA_TYPE` temporary calls `DataType`'s drop glue, which
+ // LLVM doesn't inline, on every value. A const owns no heap memory, so
+ // skipping the drop leaks nothing.
+ as_datetime_with_data_type(&ManuallyDrop::new(T::DATA_TYPE), v)
}
/// Converts a value of the given [`DataType`] to [`NaiveDateTime`]
@@ -269,7 +273,8 @@ pub(crate) fn as_datetime_with_data_type(data_type:
&DataType, v: i64) -> Option
/// Converts an [`ArrowPrimitiveType`] to [`DateTime<Tz>`]
pub fn as_datetime_with_timezone<T: ArrowPrimitiveType>(v: i64, tz: Tz) ->
Option<DateTime<Tz>> {
- as_datetime_with_timezone_and_data_type(&T::DATA_TYPE, v, tz)
+ // See `as_datetime` for why the temporary is not dropped
+ as_datetime_with_timezone_and_data_type(&ManuallyDrop::new(T::DATA_TYPE),
v, tz)
}
/// Converts a value of the given [`DataType`] to [`DateTime<Tz>`]
@@ -293,7 +298,8 @@ pub fn as_date<T: ArrowPrimitiveType>(v: i64) ->
Option<NaiveDate> {
/// Converts an [`ArrowPrimitiveType`] to [`NaiveTime`]
pub fn as_time<T: ArrowPrimitiveType>(v: i64) -> Option<NaiveTime> {
- as_time_with_data_type(&T::DATA_TYPE, v)
+ // See `as_datetime` for why the temporary is not dropped
+ as_time_with_data_type(&ManuallyDrop::new(T::DATA_TYPE), v)
}
/// Converts a value of the given [`DataType`] to [`NaiveTime`]
@@ -327,7 +333,8 @@ pub(crate) fn as_time_with_data_type(data_type: &DataType,
v: i64) -> Option<Nai
/// Converts an [`ArrowPrimitiveType`] to [`Duration`]
pub fn as_duration<T: ArrowPrimitiveType>(v: i64) -> Option<Duration> {
- match T::DATA_TYPE {
+ // See `as_datetime` for why the temporary is not dropped
+ match &*ManuallyDrop::new(T::DATA_TYPE) {
DataType::Duration(unit) => match unit {
TimeUnit::Second => try_duration_s_to_duration(v),
TimeUnit::Millisecond => try_duration_ms_to_duration(v),
