Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16036 )
Change subject: WIP [perf] KUDU-3140 Heuristics to disable predicate evaluation for Bloom filter ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/generic_iterators.cc@1290 PS2, Line 1290: num_rows_before > Shouldn't we be measuring this before the call to MaterializeColumn(), whic Thanks for pointing that out. Definitely missed it. This would entail passing down the info that the predicate evaluation must be skipped. Tagging along DecoderEvalNotSupported() as it's already used to disable decoder level evaluation. http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/predicate_effectiveness.h File src/kudu/common/predicate_effectiveness.h: http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/predicate_effectiveness.h@30 PS2, Line 30: // Per column predicate effectiveness context. : // Though all column predicate types are tracked, effectiveness of only Bloom filter predicate : // is currently enforced. > I don't agree with making this generic for all predicates. It seems like ef Done http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/predicate_effectiveness.h@38 PS2, Line 38: : type(type), rows_read(0), : rows_rejected(0), enabled(true) { > micro-nit: would prefer putting these on a new line (at least I tend to fav Done http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/predicate_effectiveness.h@83 PS2, Line 83: CheckEffectiveness > nit: it isn't really clear from this naming that this also disables the pre Done http://gerrit.cloudera.org:8080/#/c/16036/2/src/kudu/common/predicate_effectiveness.h@88 PS2, Line 88: co-ordinate > nit: coordinate Done -- To view, visit http://gerrit.cloudera.org:8080/16036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10197800a01a1b34c7821ac879caf8d272cab8dd Gerrit-Change-Number: 16036 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 11 Jun 2020 02:35:18 +0000 Gerrit-HasComments: Yes
