mapleFU commented on PR #38784: URL: https://github.com/apache/arrow/pull/38784#issuecomment-1826051384
The difference between current version and 13.0.0 version is it call more `BinaryBuilder->Reserve()`, I don't think it would make performance worse, so I revert some change in https://github.com/apache/arrow/pull/38784/commits/60cdb808c9ab6d25b2e00474a6307a8bf35b7b24 . Maybe we can considering this later. This patch also separate `PrepareNextInput`. Maybe it's a compiler specific problem, I've check a similar case in quick-bench, it doesn't benfit the performance if we remove `std::optional<>` as argument in gcc-12.3 with C++17. However benchmark shows that the performance has benefits. Maybe we can ask some C++ experts for help. You can decide how to handle this patch later, maybe I'm becoming crazy because spending long time but don't know why previously, at least we found that: 1. `Reserve` might related to the problem 2. `PrepareInput` in the patch might make compiler doesn't do some optimizations in ursa bot benchmark -- 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]
