David Ribeiro Alves has posted comments on this change. Change subject: interval_tree: allow bulk queries ......................................................................
Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/6481/4/src/kudu/util/interval_tree-inl.h File src/kudu/util/interval_tree-inl.h: PS4, Line 248: into into those PS4, Line 275: rem_queries explain what this is. http://gerrit.cloudera.org:8080/#/c/6481/4/src/kudu/util/interval_tree-test.cc File src/kudu/util/interval_tree-test.cc: PS4, Line 239: TEST_F(TestIntervalTree, TestBigO) { can you make this parameterized and do (small ints, big qs) (big ints, big qs) and (big ints small qs?) I'd be nice to see those results in the commit message http://gerrit.cloudera.org:8080/#/c/6481/4/src/kudu/util/interval_tree.h File src/kudu/util/interval_tree.h: PS4, Line 122: 1 3 PS4, Line 132: ItType begin_queries, : ItType end_queries, so the point is to not make this dependent on vector or something? feel like this is obfuscating the purpose and making it hard to read. might be worth considering simplifying. Also "begin_queries" makes it read like its a "set of begins", which can't be derived from context because of templating. I had to go look in the test to be sure what this is taking. -- To view, visit http://gerrit.cloudera.org:8080/6481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb4da25ca43413fbcae631a7b0f3f16062e4e408 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
