[kudu-CR] Inlined dispatch for predicate evaluation
Todd Lipcon has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Todd Lipcon has submitted this change and it was merged. Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator must first be determined. Batched evaluation, which calls ColumnPredicate::Evaluate(), will make a function call to the correct comparator for every row. The evaluation itself gets split into batches, but each row in the batch still makes the function call, which slows performance. To remediate this, this evaluation has been templatized so there is only a single function call per batch. Additionally, when decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell at EvaluateCell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of plain encoding for decoder-level evaluation without this templating adjustment. The query selects a one tenth the values out of a plain-encoded column containing 10M strings. EvaluateCell (the Pushdown bar) in this implementation gets compiled to a dispatched function call per row, which slows it down. Evaluate (the Normal Eval bar) also dispatches once per row. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_without_inlining.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. The comparator is known statically, so calls to EvaluateCell will be inlined. Additionally, Evaluate only dispatches once per batch. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_with_inlining.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Reviewed-on: http://gerrit.cloudera.org:8080/4164 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/9/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 312: DCHECK(sel); > nit: this will actually give an "unused result" compiler warning. Should ju Done -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#10). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator must first be determined. Batched evaluation, which calls ColumnPredicate::Evaluate(), will make a function call to the correct comparator for every row. The evaluation itself gets split into batches, but each row in the batch still makes the function call, which slows performance. To remediate this, this evaluation has been templatized so there is only a single function call per batch. Additionally, when decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell at EvaluateCell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of plain encoding for decoder-level evaluation without this templating adjustment. The query selects a one tenth the values out of a plain-encoded column containing 10M strings. EvaluateCell (the Pushdown bar) in this implementation gets compiled to a dispatched function call per row, which slows it down. Evaluate (the Normal Eval bar) also dispatches once per row. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_without_inlining.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. The comparator is known statically, so calls to EvaluateCell will be inlined. Additionally, Evaluate only dispatches once per batch. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_with_inlining.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/10 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 10: Build Started http://104.196.14.100/job/kudu-gerrit/3283/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Dan Burkert has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 9: Code-Review+1 LGTM except for Todd's comment. -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Todd Lipcon has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/9/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 312: DCHECK_NOTNULL(sel); nit: this will actually give an "unused result" compiler warning. Should just be DCHECK(sel) -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 9: Build Started http://104.196.14.100/job/kudu-gerrit/3281/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#9). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator must first be determined. The default evaluation, which calls ColumnPredicate::Evaluate(), will make a function call to the correct comparator for every row. The evaluation itself gets split into batches, but each row in the batch still makes the function call, which slows performance. To remediate this, the batch evaluation has been templatized so there is only a single function call per batch. Additionally, when decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell at EvaluateCell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of plain encoding for decoder-level evaluation without this templating adjustment. The query selects a one tenth the values out of a plain-encoded column containing 10M strings. EvaluateCell (the Pushdown bar) in this implementation gets compiled to a dispatched function call per row, which slows it down. Evaluate (the Normal Eval bar) also dispatches once per row. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_without_inlining.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. The comparator is known statically, so calls to EvaluateCell will be inlined. Additionally, Evaluate only dispatches once per batch. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_with_inlining.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/9 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/8/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: PS8, Line 312: CHECK DCHECK got lost in rebase. Will change. -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/3274/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#8). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator must first be determined. The default evaluation, which calls ColumnPredicate::Evaluate(), will make a function call to the correct comparator for every row. The evaluation itself gets split into batches, but each row in the batch still makes the function call, which slows performance. To remediate this, the batch evaluation has been templatized so there is only a single function call per batch. Additionally, when decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell at EvaluateCell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of plain encoding for decoder-level evaluation without this templating adjustment. The query selects a one tenth the values out of a plain-encoded column containing 10M strings. EvaluateCell (the Pushdown bar) in this implementation gets compiled to a dispatched function call per row, which slows it down. Evaluate (the Normal Eval bar) also dispatches once per row. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_without_inlining.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. The comparator is known statically, so calls to EvaluateCell will be inlined. Additionally, Evaluate only dispatches once per batch. https://raw.githubusercontent.com/anjuwong/kudu/695cbaa016a8e94f164105d84024ceaac4b62375/docs/images/SELECT_WHERE_EQUAL_with_inlining.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/8 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/4164/7//COMMIT_MSG Commit Message: PS7, Line 25: 1M unique strings > is this "dictionary encoded" column actually just falling back to 'plain' m Yes, the main beneficiary of this patch wrt EvaluateCell would be any encoding that has makes many calls to EvaluateCell, which is only 'plain' encoding at the moment. I'll elaborate a bit more here. http://gerrit.cloudera.org:8080/#/c/4164/7/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 312: CHECK_NOTNULL(sel); > think this could be a DCHECK Done. My thinking before was that CHECK should be used when the false case would break the code (eg here it would give a nullptr exception), but since we know this shouldn't be null based on our own code, DCHECK makes sense (particularly since this is on the hot path). -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Todd Lipcon has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/4164/7//COMMIT_MSG Commit Message: PS7, Line 25: 1M unique strings is this "dictionary encoded" column actually just falling back to 'plain' mode, though? http://gerrit.cloudera.org:8080/#/c/4164/7/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 312: CHECK_NOTNULL(sel); think this could be a DCHECK -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/3251/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#7). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator must first be determined. The default evaluation, which calls ColumnPredicate::Evaluate(), will make a function call to the correct comparator for every row. The evaluation itself gets split into batches, but each row in the batch still makes the function call, which slows performance. To remediate this, the batch evaluation has been templatized so there is only a single function call per batch. Additionally, when decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings with decoder-level evaluation enabled. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/7 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#6). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator given the physical type of interest must first be determined. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. When decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 57 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/6 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/3248/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3236/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#5). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator given the physical type of interest must first be determined. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. When decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 59 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/5 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/4164/4/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: PS4, Line 331: headers > I think this would be more accurately worded as "Instantiate template for a Done http://gerrit.cloudera.org:8080/#/c/4164/4/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 142: void EvaluateForPhysicalType(const ColumnBlock& block, > Can this be private? Done Line 221: // // Templated evaluation will inline the dispatch. > left over? Done -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Dan Burkert has posted comments on this change.
Change subject: Inlined dispatch for predicate evaluation
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4164/4/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
PS4, Line 331: headers
I think this would be more accurately worded as "Instantiate template for all
physical types."
http://gerrit.cloudera.org:8080/#/c/4164/4/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:
Line 142: void EvaluateForPhysicalType(const ColumnBlock& block,
Can this be private?
Line 147: bool EvaluateCell(const void* cell) const {
This body can be defined in the cc file. It's only necessary to put the
definition in the header if you are not or can not pre-declare all the template
specializations.
Line 221: // // Templated evaluation will inline the dispatch.
left over?
--
To view, visit http://gerrit.cloudera.org:8080/4164
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3226/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#4). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, the correct comparator given the physical type of interest must first be determined. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. When decoder-level evaluation is enabled, rather than occuring in batches, dispatch occurs for each cell, which leads to poorer performance. To remediate this, the dispatch has been templatized in hopes that the dispatching and branching are inlined. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot below, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 3 files changed, 102 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/4 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#3). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, we must first determine the correct comparator to handle the physical type of interest. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch occurs for each cell. This particularly slows down decoder-level evaluation, which falls back on evaluating cell-by-cell rather than in batches. To remediate this, the dispatch has been templatized here to remediate the cost of repeated dispatch. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot on the right, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 103 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/3 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3223/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/2//COMMIT_MSG Commit Message: > Does it make sense to add some unit tests for the new methods/functions? O The behavior is covered by tablet-decoder-eval-test, which exercise EvaluateCell for dictionary and plain encoding. Currently, the non-BINARY branches are not used and not tested, since decoder-level evaluation is only supported right now for dictionary/plain encoding (both BINARY), so it may be worth testing those branches (or maybe taking them out). -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4164/2//COMMIT_MSG Commit Message: Line 6: line width: 70 http://gerrit.cloudera.org:8080/#/c/4164/2/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 142: bool EvaluateCell(DataType physical_type, const void *cell) const; > I think you may want to define this method in the header, so that you can b Will try out the public instantiations as discussed this morning. -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Alexey Serbin has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/2//COMMIT_MSG Commit Message: Does it make sense to add some unit tests for the new methods/functions? Or it's already covered enough with existing set of tests? -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Todd Lipcon has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/2/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 142: bool EvaluateCell(DataType physical_type, const void *cell) const; > I think you may want to define this method in the header, so that you can b hrm, yea if the callers are from outside this header, then you do need to include the definition in the header, or in a separate "column_predicate-inl.h" which would be included by anyone who needs the definition. Looking at the assembly output with gdb is probably a good idea to make sure it's inlining as expected -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Dan Burkert has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/2/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 142: bool EvaluateCell(DataType physical_type, const void *cell) const; I think you may want to define this method in the header, so that you can be absolutely sure that the switch gets inlined away (since you are always calling it with a statically known DataType). The other option is to just expose it as a templatized method, but you will have to add public instantiations, I think. Todd, what do you think? -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3142/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#2). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, we must first determine the correct comparator to handle the physical type of interest. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch occurs for each cell. This particularly slows down decoder-level evaluation, which falls back on evaluating cell-by-cell rather than in batches. To remediate this, the dispatch has been templatized here to remediate the cost of repeated dispatch. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot on the right, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 83 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/2 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3139/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4164 Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, we must first determine the correct comparator to handle the physical type of interest. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch occurs for each cell. This particularly slows down decoder-level evaluation, which falls back on evaluating cell-by-cell rather than in batches. To remediate this, the dispatch has been templatized here to remediate the cost of repeated dispatch. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot on the right, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 86 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/1 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong
