acruise commented on issue #6765:
URL:
https://github.com/apache/datafusion-comet/issues/6765#issuecomment-6582739762
I implemented this as described: `ArrayWriter`, `StructWriter` and
`MapWriter` each keep one
`UnsafeArrayData` / `UnsafeRow` / `UnsafeMapData` and point it at each
value, and `ArrowWriter`
collects those views at construction (by type, so each is detached through a
direct call, not a
virtual one) and points them at an empty layout after every `write` and in
`finish`. A flat schema
collects none. The reachability test passes with the detach and fails
without it, for unsafe rows
and for generic rows holding unsafe values, with and without nulls.
The reuse saves what this issue measured, but the per-row detach costs about
the same, so against
`main` the net is nil, and `array<map<...>>` loses to the inlining quirk in
#6757.
Throwaway harness: one column of 8192 unsafe rows per shape, every tenth row
and element null,
collections of `i % 6` elements, all ten shapes warmed together in one JVM,
best of 30 rounds after
10 warmup rounds, median of three JVMs. Apple M3 Max, assertions off (`-ea`
removed from the test
argLine). ns per row:
| shape | main (21) | reuse, no detach (21) | reuse + per-row detach (21) |
main (17) | reuse, no detach (17) | reuse + per-row detach (17) |
|---|---|---|---|---|---|---|
| int | 5.8 | 5.9 | 6.8 | 7.0 | 6.9 | 6.5 |
| string | 12.2 | 11.4 | 12.6 | 12.2 | 12.0 | 11.8 |
| array<string> | 28.5 | 25.9 | 29.1 | 30.0 | 27.4 | 27.2 |
| map<string,string> | 50.3 | 47.8 | 52.4 | 52.7 | 49.1 | 50.5 |
| array<struct<int,string>> | 55.4 | 53.4 | 57.0 | 58.2 | 53.9 | 55.3 |
| array<array<int>> | 44.0 | 41.8 | 45.7 | 44.7 | 43.3 | 45.6 |
| map<int,long> | 29.5 | 27.5 | 30.6 | 29.7 | 27.6 | 31.5 |
| struct<int,string> | 21.9 | 20.8 | 22.0 | 22.0 | 20.3 | 22.1 |
| array<map<string,int>> | 97.8 | 111.2 | 112.6 | 99.4 | 94.4 | 121.0 |
| row of 8 (int, long, string, double, array, map, struct, binary) | 119.6 |
114.4 | 124.0 | 124.4 | 116.6 | 124.7 |
(21) is JDK 21.0.12, (17) is JDK 17.0.19. Run-to-run spread is about 2 ns on
the nested shapes.
Reading it:
- Reuse without any detach, which is what #6730 had before 0623dbac51, is 2
to 4 ns per row faster
than `main` on the nested shapes and 5 to 8 on the wide row, on both JDKs.
That matches the table
in this issue.
- Detaching once per row gives all of that back. One `pointTo` per nested
writer per row is 1 to 3
ns, the same order as the saving, and with four nested writers in the wide
row it is a wash. On
JDK 21 every nested shape is at or slightly above `main`; on JDK 17 three
are 2 to 3 ns under,
two are 1 to 2 ns over, the rest are equal.
- `array<map<string,int>>` is 15 to 20% slower with the reuse on both JDKs,
with or without the
detach. That is the C2 inlining quirk in #6757, which the view-per-value
code on `main` happens
to avoid.
The first point is the only version that pays, and it keeps up to one row
reachable per nested
writer until the batch's `finish`, which is what the #6730 review rejected.
Since a per-row detach
costs what the reuse saves, the next thing to try is detaching once per
batch instead: at `finish`
and `reset`, with the same views. That bounds the retention to one row per
nested writer for the
lifetime of one batch, at no per-row cost, and gives the "reuse, no detach"
column above. I'll
measure that and the `array<map>` case before opening anything.
--
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]
