Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17389 )
Change subject: IMPALA-10680: Replace StringToFloatInternal using fast_double_parser library ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h File be/src/util/string-parser.h: http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h@545 PS6, Line 545: std::string res; > I wonder if the performance numbers are still the same, given that now we n I had checked Small String Optimization and had read till 24 bytes it would not do allocation which appeared reasonable. But i rechecked after your comment and 24 bytes seem to be only for clang. I wrote a small program to check for our g++ and it avoids allocation till 15 bytes only. Program: #include <cstdlib> #include <iostream> #include <string> // replace operator new and delete to log allocations void* operator new(std::size_t n) { std::cout << "[Allocating " << n << " bytes]"; return malloc(n); } void operator delete(void* p) throw() { free(p); } int main() { for (size_t i = 0; i < 30; ++i) { std::cout << i << ": " << std::string(i, '=') << std::endl; } } is 15 good enough length ? otherwise dynamic allocation will make things slow. My assumption is input won't be long enough to cause stack overflow. I am not sure if we have some limitation on the string being passed to StringToFloat. -- To view, visit http://gerrit.cloudera.org:8080/17389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic105ad38a2fcbf2fb4e8ae8af6d9a8e251a9c141 Gerrit-Change-Number: 17389 Gerrit-PatchSet: 6 Gerrit-Owner: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 21 May 2021 10:49:09 +0000 Gerrit-HasComments: Yes