[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. remove last vestiges of boost::bind, boost::function, and std::bind With this patch, all Kudu functors use std::function, and all capturing or binding is done via C++ lambdas. The lack of support for capture-via-move rears its ugly head in raft_consensus.cc, but I verified that this only leads to inefficiency, not incorrectness (i.e. lifecycle issues). C++14 (or 17) support can't come soon enough! Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Reviewed-on: http://gerrit.cloudera.org:8080/15639 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.h M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client-unittest.cc M src/kudu/client/client.cc M src/kudu/client/master_proxy_rpc.cc M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/session-internal.cc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/reactor-test.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/response_callback.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc.cc M src/kudu/rpc/rpc.h M src/kudu/rpc/rpc_stub-test.cc M src/kudu/security/init.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/server_base.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/server/webserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_path_handlers.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/url-coding.cc M src/kudu/util/web_callback_registry.h 57 files changed, 522 insertions(+), 550 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:43:31 + Gerrit-HasComments: No
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > I think that's already implied in the existing comment, no? "libev callback Indeed, that information can be easily derived from current comment. http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > See above. Ack. -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:02:17 + Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > nit: while you are here, maybe add a comment that the signature of this fun I think that's already implied in the existing comment, no? "libev callback for handling ...". http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > ditto See above. -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 21:12:04 + Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/client/client.cc@1684 PS1, Line 1684: // CloseCallback::Callback() deletes the closer. Thank you for adding this comment. http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > warning: non-const reference parameter 'watcher', make it const or use a po nit: while you are here, maybe add a comment that the signature of this function is dictated by libev? http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > warning: non-const reference parameter 'watcher', make it const or use a po ditto -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 20:26:20 + Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15639 to review the following change. Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. remove last vestiges of boost::bind, boost::function, and std::bind With this patch, all Kudu functors use std::function, and all capturing or binding is done via C++ lambdas. The lack of support for capture-via-move rears its ugly head in raft_consensus.cc, but I verified that this only leads to inefficiency, not incorrectness (i.e. lifecycle issues). C++14 (or 17) support can't come soon enough! Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.h M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client-unittest.cc M src/kudu/client/client.cc M src/kudu/client/master_proxy_rpc.cc M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/session-internal.cc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/reactor-test.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/response_callback.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc.cc M src/kudu/rpc/rpc.h M src/kudu/rpc/rpc_stub-test.cc M src/kudu/security/init.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/server_base.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/server/webserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_path_handlers.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/url-coding.cc M src/kudu/util/web_callback_registry.h 57 files changed, 522 insertions(+), 550 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/15639/1 -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong