[GitHub] [arrow] jgehrcke commented on pull request #33684: GH-15171: [C++] Pass std::string_view by value

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

jgehrcke commented on PR #33684:
URL: https://github.com/apache/arrow/pull/33684#issuecomment-1419225622

   Some curiosity-driven feedback CC @ucasfl @westonpace @pitrou. 
   
   I read https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/ with curiosity and was wondering if this patch here resulted in a measurable performance difference somewhere on the user-facing surface of Arrow.
   
   Before Christmas I build a benchmark that I called `dataset-serialize`. Among others, it measures the time for dataset CSV serialization, writing to memory (/dev/shm). It also runs on bare metal. Both in combination yields quite some stability.
   
   When this commit landed, said benchmark got faster by (roughly) three percent:
   
   ![image](https://user-images.githubusercontent.com/265630/217006190-5f33a7fb-f0b5-40a9-98ef-82787e1b4f08.png)
   
   The plot/data can be further explored here: https://conbench.ursa.dev/benchmarks/bc6d37ffe2614078814efb01402f64dd/
   
   Of course this is just correlation (once this was merged, the benchmark got a tad faster) and not necessarily causation (for corroborating this I understand too little of the impact of this change). But yeah, it's probably fair to say that it's not unlikely that this change here was responsible for the perf change.


-- 
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@arrow.apache.org

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