[Impala-ASF-CR] IMPALA-10323: use gutil to cast int to string
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7722/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 12:07:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10323: use gutil to cast int to string
Csaba Ringhofer has submitted this change and it was merged. ( 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 Reviewed-on: http://gerrit.cloudera.org:8080/16719 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins Reviewed-by: Csaba Ringhofer --- M be/src/exprs/cast-functions-ir.cc 1 file changed, 34 insertions(+), 16 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified Csaba Ringhofer: Looks good to me, approved -- 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: merged Gerrit-Change-Id: Ief1d6b2c2cb5961c1d6cee1b0eac86ab61509768 Gerrit-Change-Number: 16719 Gerrit-PatchSet: 8 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 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