This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11896-09bb64ac5c5221a927601d43c4e93e8e6e5c4eff in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
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),
