[Impala-ASF-CR] IMPALA-10323: use gutil to cast int to string
wesleydeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16719 ) Change subject: IMPALA-10323: use gutil to cast int to string .. Patch Set 7: (3 comments) > wesleydeng, can you publish the change as non-draft? The later > patches are drafts, which is why the Jenkins job didn't merge it. done http://gerrit.cloudera.org:8080/#/c/16719/3/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/16719/3/be/src/exprs/cast-functions-ir.cc@134 PS3, Line 134: > I'd prefer if you used a constant here, i.e. MAX_FLOAT_CHARS and maybe add Actually max_char_size is a constant here which will be replace by MAX_BIGINT_CHARS. But to defend others to reuse this macro by passing a variable, I will make this modification as you suggest. I has some problem to connect gerrit now, I will push new patch on Monday. http://gerrit.cloudera.org:8080/#/c/16719/3/be/src/exprs/cast-functions-ir.cc@140 PS3, Line 140: char char_buffer[MAX_EXACT_NUMERIC_CHARS];\ : char* end_position = to_string_method(val.val, char_buffer); \ : size_t len = end_position - char_buffer; \ : DCH > I wonder if we can skip this check, as a StringVal object with not sufficie I keep this check because I'm not sure. Other sugguestion are applied to new patch. http://gerrit.cloudera.org:8080/#/c/16719/4/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/16719/4/be/src/exprs/cast-functions-ir.cc@27 PS4, Line 27: #include > nit: we generally include headers sorted alphabetically, so numbers.h comes fixed -- To view, visit http://gerrit.cloudera.org:8080/16719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 Gerrit-Change-Number: 16719 Gerrit-PatchSet: 7 Gerrit-Owner: wesleydeng Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: abeltian Gerrit-Reviewer: wesleydeng Gerrit-Comment-Date: Tue, 24 Nov 2020 11:45:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10323: use gutil to cast int to string
wesleydeng has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/16719 ) Change subject: IMPALA-10323: use gutil to cast int to string .. IMPALA-10323: use gutil to cast int to string Using gutil instead of lexical_cast when casting int type value to string, performance can be improved significantly in multi-thread mode. In contrast, performance using lexical_cast decreased seriously in multi-thread mode. For example: when mt_dop=8, this change can get 20x performance improvement; when mt_dop=0, this change can get 2x performance improvement; Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 --- M be/src/exprs/cast-functions-ir.cc 1 file changed, 34 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/16719/7 -- To view, visit http://gerrit.cloudera.org:8080/16719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 Gerrit-Change-Number: 16719 Gerrit-PatchSet: 7 Gerrit-Owner: wesleydeng Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: abeltian Gerrit-Reviewer: wesleydeng
[Impala-ASF-CR] IMPALA-10323: use snprintf instead of lexical cast to cast to string
wesleydeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16719 ) Change subject: IMPALA-10323: use snprintf instead of lexical_cast to cast to string .. Patch Set 1: (1 comment) > (1 comment) use buffer may result in lock contention in multi-thread mode where I want to improve performance. To save memory, I have another proposal: May be we can define 3 level char size : 5,10,20, for bigint value , if value<999 && value > -9, then change char_size to 10; else if value <9 && value>-, than change char size to 5; http://gerrit.cloudera.org:8080/#/c/16719/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16719/1//COMMIT_MSG@10 PS1, Line 10: performance can be improved significantly in : multi-thread mode(mt_dop mode). In contrast, performance using : lexical_cast decreased seriously in multi-thread mode. > Do you have any measurements to see what is the order of magnitude of this As it show in IMPALA-10323, use snprintf can save 90% time compared with using lexical_cast in mt_dop=8 mode -- To view, visit http://gerrit.cloudera.org:8080/16719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 Gerrit-Change-Number: 16719 Gerrit-PatchSet: 1 Gerrit-Owner: wesleydeng Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: abeltian Gerrit-Reviewer: wesleydeng Gerrit-Comment-Date: Tue, 17 Nov 2020 13:18:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10323: use snprintf instead of lexical cast to cast to string
wesleydeng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16719 Change subject: IMPALA-10323: use snprintf instead of lexical_cast to cast to string .. IMPALA-10323: use snprintf instead of lexical_cast to cast to string Like issue IMPALA-1738, using snprintf instead of lexical_cast when casting to string, performance can be improved significantly in multi-thread mode(mt_dop mode). In contrast, performance using lexical_cast decreased seriously in multi-thread mode. Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 --- M be/src/exprs/cast-functions-ir.cc 1 file changed, 23 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/16719/1 -- To view, visit http://gerrit.cloudera.org:8080/16719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 Gerrit-Change-Number: 16719 Gerrit-PatchSet: 1 Gerrit-Owner: wesleydeng
