Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12196 )
Change subject: generic_iterators: prep for MergeIterator dominance ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/12196/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12196/2//COMMIT_MSG@9 PS2, Line 9: This patch adds a histogram to the MergeIterator that tracks the number of > instead of a histogram isn't what we really care about just the total numbe I originally went with a histogram because in addition to the total number of comparisons, I also wanted to get a sense for how they were distributed across the lifetime of the MergeIterator. Spitting out the percentiles as I did doesn't really accomplish that, nor does the distribution really matter for the purposes of computing the increase in efficiency (we assume that every scan runs to completion, so the total CPU consumption is the only thing that really matters). Anyway, I'll switch this to a raw counter. http://gerrit.cloudera.org:8080/#/c/12196/1/src/kudu/common/generic_iterators.h File src/kudu/common/generic_iterators.h: http://gerrit.cloudera.org:8080/#/c/12196/1/src/kudu/common/generic_iterators.h@184 PS1, Line 184: HdrHistogram sub_iter_histo_; > I see the appeal of this for your testing, but does this have adverse perf I don't know; I didn't measure the impact of the histogram itself. Looking at the implementation, it's conceivable that it may have an impact, as it does some arithmetic, atomic increments, and CASes. Are you open to a counter, even if it was only used in tests? IteratorStats would probably be the best home, but those are maintained on a per-column basis; "number of key comparisons" doesn't really make sense in that context, unless you want it broken up by key column. And beyond that, it's a metric that very clearly only matters if using ORDERED scans. -- To view, visit http://gerrit.cloudera.org:8080/12196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a8db5973dc09b7271e05b3cc28025b7a2a9e21b Gerrit-Change-Number: 12196 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 11 Jan 2019 23:53:50 +0000 Gerrit-HasComments: Yes
