[I] perf: converting unsafe arrays of maps to Arrow is up to 1.7x slower when C2 inlines MapWriter.writeEntries [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] perf: converting unsafe arrays of maps to Arrow is up to 1.7x slower when C2 inlines MapWriter.writeEntries [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove commented on issue #6757:
URL: https://github.com/apache/datafusion-comet/issues/6757#issuecomment-6048374458

   #6730 no longer reuses the views (0623dbac51). `MapWriter.writeArrayElements` now calls `writeEntries(array.getMap(i))`, so the `writeUnsafe` helper above is gone, but the quirk is still there. In the same harness, now warming 22 cases together, `array<map<string,int>>` takes 143 ns per row, and 108 with `-XX:CompileCommand=dontinline,...MapWriter::writeEntries` (two runs each). `main` takes 202 on it, so it is still faster than `main`, but about 1.3x slower than it could be.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org