Qifan Chen 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; > Yeah, I think it'd be good to see measurements for longer float literals. W Stack overflow is sensitive to the depth of the stack and the size of the current frame. Assume in most cases, the input string conforms to the format that the fast double parser library can handle properly, then we may not need to build the string object at all. Thus, we can use the following form of signature: inline bool stringToFloatPreprocess(char*, int offset, int len, string* preprocessed_result); We return string (in preprocessed_result) only when the input does not conform. In good case, we do not construct a string object and we can feed the original input string in StringToFloatInternal() to the fast parser library, if the input string is null-terminated. In seems this is the case as the only use of the StringToFloatInternal() is called indirectly here throughout Impala BE. 343 static bool ParseProbability(const string& prob_str, bool* should_execute) { 344 StringParser::ParseResult parse_result; 345 double probability = StringParser::StringToFloat<double>( 346 prob_str.c_str(), prob_str.size(), &parse_result); 347 if (parse_result != StringParser::PARSE_SUCCESS || 348 probability < 0.0 || probability > 1.0) { 349 return false; 350 } 351 // +1L ensures probability of 0.0 and 1.0 work as expected. 352 *should_execute = rand() < probability * (RAND_MAX + 1L); 353 return true; 354 } -- 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 15:27:08 +0000 Gerrit-HasComments: Yes
