alamb opened a new issue, #10889:
URL: https://github.com/apache/arrow-rs/issues/10889

   
   **Component(s)**: arrow-array
   
   ## Is your feature request related to a problem or challenge?
   
   While surveying the DataFusion workspace for binary-size opportunities with 
`cargo llvm-lines`
   - https://github.com/apache/datafusion/issues/24727
   
   The single largest generic-code item in *every* crate I measured was not 
DataFusion code at all — it was the `Debug` impl for `PrimitiveArray<T>`:
   
   Specifically:
   | Function | IR lines | Instantiations |
   |---|---:|---:|
   | `<PrimitiveArray<T> as Debug>::fmt::{closure#0}` | 47,299 | 32 |
   | `print_long_array::<PrimitiveArray<T>, {closure}>` | 29,472 | 32 |
   | `<PrimitiveArray<T> as Debug>::fmt` | 7,712 | 32 |
   | `temporal_conversions::as_time` (pulled in by the closure) | 5,312 | 32 |
   | `temporal_conversions::as_datetime` (pulled in by the closure) | 3,136 | 
32 |
   
   That is ~85,000 IR lines (~4-6% of each crate's total codegen), and because 
generic functions are instantiated in the *consuming* crate, the same ~85K 
lines are generated again in every crate that debug-formats a `PrimitiveArray` 
— directly or indirectly. `Debug` is reachable from 
`unwrap`/`expect`/`assert_eq!` and error formatting, so essentially every crate 
that touches Arrow arrays pays this.
   
   
   ## What is going on
   
   The `Debug` impl is generic over `T: ArrowPrimitiveType` and is 
monomorphized for all ~32 primitive types:
   
   
https://github.com/apache/arrow-rs/blob/59.2.0/arrow-array/src/array/primitive_array.rs#L1342-L1412
   
   ```rust
   impl<T: ArrowPrimitiveType> std::fmt::Debug for PrimitiveArray<T> {
       fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
           let data_type = self.data_type();
   
           write!(f, "PrimitiveArray<{data_type}>\n[\n")?;
           print_long_array(self, f, |array, index, f| match data_type {
               DataType::Date32 | DataType::Date64 => { /* as_date::<T> ... */ }
               DataType::Time32(_) | DataType::Time64(_) => { /* as_time::<T> 
... */ }
               DataType::Timestamp(_, tz_string_opt) => {
                   /* timezone parsing, as_datetime_with_timezone::<T>, RFC3339 
formatting,
                      error messages for invalid timezones ... */
               }
               _ => std::fmt::Debug::fmt(&array.value(index), f),
           })?;
           write!(f, "]")
       }
   }
   ```
   
   The per-value closure contains the entire temporal-formatting `match` — 
date/time conversion, timezone parsing, RFC3339 formatting — and because the 
`match` is on the *runtime* value `self.data_type()`, the compiler cannot prune 
those arms even for types like `Int8Type` that can never be temporal. Every one 
of the 32 instantiations carries all of it.
   
   In addition, `print_long_array` is generic over both the array type and the 
closure type, so the head/tail truncation logic is also duplicated 32 times:
   
   
https://github.com/apache/arrow-rs/blob/59.2.0/arrow-array/src/array/mod.rs#L1045-L1080
   
   The only genuinely type-specific code in all of this is the default arm: 
`Debug::fmt(&array.value(index), f)` for `T::Native` — a handful of lines. 
Everything else is identical across instantiations.
   
   ## Describe the solution you'd like
   
   Restructure so the shared machinery is compiled once, keeping only the tiny 
per-native-type value formatting monomorphized. For example:
   
   - Change `print_long_array` to take `&dyn Array` and `&mut dyn FnMut(...)` 
instead of being generic over `A` and `F` (it is called through `Array` trait 
methods only), and/or
   - Move the temporal `match` out of the per-value closure into a non-generic 
helper: select a formatting function by matching `data_type` *once*, then have 
the generic `fmt` pass only the trivial `Debug::fmt(&value)` closure for the 
default case.
   
   `Debug` formatting is not performance-sensitive, so the usual 
monomorphization-for-speed argument does not apply; this should be pure 
code-size win with no measurable cost. A similar runtime-dispatch restructuring 
in DataFusion (apache/datafusion#24687) removed 67% of a module's generated 
code with benchmark-verified zero runtime impact.
   
   The same `print_long_array` pattern is used by other `Debug` impls 
(`GenericByteArray`, `GenericListArray`, etc.); those have far fewer 
instantiations, but would benefit from the same non-generic helper.
   
   ## Describe alternatives you've considered
   
   Leaving it as is: downstream binaries pay a few hundred KB and every crate 
compiling against arrow-array spends LLVM time on ~85K lines of duplicate IR.
   
   ## Additional context
   
   Numbers above are from `cargo llvm-lines --release -p <crate> --lib` 
(cargo-llvm-lines 0.4.41) on DataFusion crates using arrow-array 59.2.0. 
Reproduce with:
   
   ```bash
   cargo install cargo-llvm-lines
   cargo llvm-lines --release -p datafusion-physical-expr --lib | grep -E 
"print_long_array|PrimitiveArray.*Debug"
   ```
   
   - Found while working on reducing DataFusion binary size: 
https://github.com/apache/datafusion/issues/24727
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to