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; > Original input string in StringToFloatInternal() is not null-terminated str Results: 3 Runs with 20 length string: ---------------------------- With std::string Fetched 1100000 row(s) in 30.54s Fetched 1100000 row(s) in 30.68s Fetched 1100000 row(s) in 30.62s With VLA: Fetched 1100000 row(s) in 30.60s Fetched 1100000 row(s) in 30.64s Fetched 1100000 row(s) in 30.56s 3 Runs with 30 length string: ----------------------------- With std::string Fetched 1100000 row(s) in 33.85s Fetched 1100000 row(s) in 33.89s Fetched 1100000 row(s) in 34.19s With VLA: Fetched 1100000 row(s) in 33.78s Fetched 1100000 row(s) in 33.71s Fetched 1100000 row(s) in 33.57s Summary: I think we are fine with performance. In real system hopefully we don't see too many tcmalloc contention due to this. Couple of things will help us avoid malloc in current scenario: 1. Strings less than 16 length due to Small string optimisation. 2. Based on Qifan's suggestion, we can avoid creating copy of string for some cases where we have non-integer terminated string as original input. Other alternative (also suggested by Zoltan) is to do VLA for string of less than 256 bytes and above that we can use strtod. I am ok with either approach. -- 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 <[email protected]> Gerrit-Reviewer: Amogh Margoor <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 21 May 2021 21:37:06 +0000 Gerrit-HasComments: Yes
