Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15122 )
Change subject: [client] KUDU-2483 Add IN Bloom filter predicate to C++ client ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/client.cc@999 PS5, Line 999: bf_it = bloom_filters->erase(bf_it); This seems unnecessarily expensive as it'll resize the vector, worse because you're iterating from front to back and not back to front (i.e. it'll shift everything down with each call to erase()). Instead, how about just nulling the pointers as you move them? 'delete null' is a safe no-op. http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/hash-internal.h File src/kudu/client/hash-internal.h: http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/hash-internal.h@17 PS5, Line 17: : #ifndef KUDU_CLIENT_HASH_INTERNAL_H : #define KUDU_CLIENT_HASH_INTERNAL_H You can use #pragma once here. http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/hash.h File src/kudu/client/hash.h: http://gerrit.cloudera.org:8080/#/c/15122/5/src/kudu/client/hash.h@26 PS5, Line 26: enum KUDU_EXPORT HashAlgorithm { Don't need KUDU_EXPOR There. -- To view, visit http://gerrit.cloudera.org:8080/15122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4aa235a4c933ebce0bf3ec7fcb135098eccc4ea4 Gerrit-Change-Number: 15122 Gerrit-PatchSet: 5 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 08 Feb 2020 06:05:35 +0000 Gerrit-HasComments: Yes
