pitrou commented on PR #39701: URL: https://github.com/apache/arrow/pull/39701#issuecomment-1900544902
> It seems that the regression results of the benchmark have nothing to do with my changes. That's true, but there doesn't seem to be any improvement either. I've run the benchmarks locally and neither do I see any improvement. We can try to reason on the code changes here: * the original code allocates the temporary buffer once at the beginning of the sort operation, and reuses it for all `std::merge` calls * the changed code lets `std::inplace_merge` allocate a new temporary buffer at each invocation So, at least theoretically, the code in this PR is less efficient. Unless you can exhibit benchmark improvements on some configuration, I would recommend rejecting this. -- 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]
