andygrove opened a new issue, #6765:
URL: https://github.com/apache/datafusion-comet/issues/6765

   ### What is the problem the feature request solves?
   
   `ArrowWriter`'s row path reads each array, map and struct of an `UnsafeRow` 
through Spark's getters, which allocate a view of the value for each value: an 
`UnsafeArrayData`, an `UnsafeMapData` with its two arrays, or an `UnsafeRow`. 
An earlier revision of #6730 pointed one reused view per writer at every value 
instead. In a harness that warms every type up together, that was 1 to 4 ns per 
row faster on nested types:
   
   | ns per row | `main` | reused views | a view per value |
   |---|---|---|---|
   | array<string> | 59.1 | 26.8 | 30.1 |
   | map<string,string> | 116.9 | 49.7 | 53.5 |
   | array<struct<int,string>> | 86.8 | 57.8 | 61.1 |
   | array<array<int>> | 80.1 | 45.6 | 47.8 |
   | map<int,long> | 49.5 | 30.2 | 31.9 |
   | struct<int,string> | 32.0 | 22.2 | 23.0 |
   
   #6730 dropped the reuse because a reused view keeps the last row it read 
reachable after the row's values are copied, along with the row's memory, until 
the writer is dropped. That is at most one row per nested writer for the life 
of a batch's writer, but with large rows it is heap that Spark does not account 
for.
   
   ### Describe the potential solution
   
   Reuse the views and detach them once per row, rather than after every value. 
Detaching after every value, by pointing each view at an empty layout, costs as 
much as it saves. It was 13% slower than a view per value on array<array<int>>, 
because an array of nested values detaches once per element. Instead, 
`ArrowWriter` could keep the nested writers that hold a view and detach just 
those after each `write`, so flat rows pay nothing. An `UnsafeArrayData` can 
point at `long[] {0}` with a size of 8, an `UnsafeMapData` at `long[] {8, 0, 
0}` with a size of 24, and an `UnsafeRow` at `(null, 0, 0)`. A null base would 
crash the first two, because their `pointTo` reads a header.
   
   `CometArrowWriterSuite`'s `the row path keeps no reference to an unsafe row 
once it is written` checks that no row stays reachable from the writers, and it 
has to keep passing.
   
   ### Additional context
   
   Measured on an M3 Max with AppleJDK 17: one column of 8192 rows per case, 
every tenth row and element null, the median of three alternating JVMs. Part of 
#6565. See the review of #6730.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to