andygrove commented on issue #6806: URL: https://github.com/apache/datafusion-comet/issues/6806#issuecomment-6195653652
@comphead I know you offered to work on this, so I wanted to let you know that I opened #7016 for it. It takes the first approach from your list: the tasks of an executor that read the same broadcast at the same time share one decoded copy, so only the first of them decodes it. On your shape (a 1M-row build side and 32 probe tasks under `local[8]`), the probe tasks' summed CPU went from 879 ms to 331 ms. It also replaces the `Channels.newChannel` loop in the decode path, though that only saved about 2.5% per decode in my runs. I left Comet's codec layer alone. With the decode shared, it is paid once per executor while its tasks overlap rather than once per task, and dropping it would roughly double the broadcast held on-heap and change the `dataSize` that AQE reads. Each task also still builds its own hash table, which is the remaining part of #6013. Would you mind taking a look? If you still have `CometHashJoinTaskTimeBenchmark` around, it would be good to see how much of the gap in your profile it closes. -- 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]
