andygrove opened a new issue, #6757: URL: https://github.com/apache/datafusion-comet/issues/6757
### Describe the bug With #6730, `ArrowWriter` converts an `UnsafeRow`'s array of maps through `MapWriter.writeArrayElements`, which points a reused `UnsafeMapData` at each element in turn and writes its keys and values through `writeEntries`. In a JVM that has also converted other nested shapes, this path is up to 1.7x slower than it needs to be. The slowdown appears when C2 inlines `writeEntries` into the loop over the array's maps. These are the times for one `array<map<string,int>>` column of 8192 rows, every tenth row and element null, with collections of `i % 6` elements. Each is the median of three alternating JVMs, each run's best: | JVM | ns per row | |---|---| | 14 shapes warmed together, default flags (four orders of first use) | 120 to 178 | | the same, `-XX:CompileCommand=dontinline,...MapWriter::writeEntries` | 102 to 104 | | the same, `-XX:CompileCommand=dontinline,...MapWriter::writeUnsafe` | 103 to 104 | | only `array<map<string,int>>` warmed | 102 | | `main`'s writer, 14 shapes warmed together | 211 | Nothing is deoptimized while it runs. The compiled `writeArrayElements` inlines everything except the string key writer, which is already compiled into a big method, and the `reAlloc` paths. `-XX:-UseTypeSpeculation` (158) and `-XX:-TieredCompilation` (116) recover part of the gap. `LoopStripMiningIter=0`, `-LoopUnswitching`, `-PartialPeelLoop`, `LoopUnrollLimit=0`, `-UseSuperWord`, `-UseLoopPredicate`, `-SplitIfBlocks` and `-UseCountedLoopSafepoints` do not. Moving the nested writers' `writeArrayElements` into one shared base class also gets 103, because that call site goes megamorphic and stops the inlining. It costs 2-10% on the other nested shapes, though, so #6730 keeps a copy per writer. The next step is to look at the compiled code with hsdis or async-profiler, neither of which was available where this was measured, to see what makes the inlined loop slow, and then to restructure `MapWriter.writeArrayElements` to avoid it. ### Steps to reproduce A throwaway harness converted 14 cases (int, string, decimal(18,2), two structs, five arrays, three maps and a row of 8 columns), each 8192 unsafe rows, warming all of them together before timing any. A and B variants ran in alternating JVMs. This was on an M3 Max with AppleJDK 17.0.10. ### Additional context Part of #6565. -- 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]
