Zoltan Borok-Nagy 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: (3 comments) I am happy with the code as is, but seems like there are still some unresolved comment threads in 'string-parser.h'. Can we resolve (or mark as Done / won't resolve) each before moving forward? 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; > Results: In a message at Jun 3 you mentioned "So I will use VAC for shorter strings to avoid malloc and for larger strings we can use malloc + strtod." Is there still a plan to do VLA for short strings? http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h@550 PS6, Line 550: > Here before copying the test of the string, make the following check and re This would probably prevent "return value optimization", as there would be multiple return statements with different objects. http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h@552 PS6, Line 552: // string has `move` defined, which would be used instead of copy. > nit: I don't think we need this comment. And I think the situation is even Do you plan to resolve this comment? -- 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: Wed, 07 Jul 2021 08:38:42 +0000 Gerrit-HasComments: Yes
