You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/12/21 00:26:33 UTC

[kudu-CR] protoc-gen-krpc: remove kudu util dependency and clean up

Hello Alexey Serbin, Andrew Wong, Volodymyr Verovkin, Todd Lipcon,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/14942

to review the following change.


Change subject: protoc-gen-krpc: remove kudu_util dependency and clean up
......................................................................

protoc-gen-krpc: remove kudu_util dependency and clean up

We've had a longstanding issue in incremental builds where a change to
versions_defines.h would trigger a rebuild of what seemed like the entire
codebase. I've looked into it at various times, enough to enumerate the set
of events that change version_defines.h:
- Rebasing on top of another commit.
- Committing a new commit.
- Taking the working tree from "dirty" to "clean" or vice versa (e.g. saving
  one edit, or running "git checkout --").

I thought I could mitigate that by removing the "clean repo" aspect of
version_defines.h, but my workflow involves the creation of many commits
with the occasional rebase to squash them, so I wouldn't benefit much.

I took a deeper look today and learned something new: protoc-gen-krpc's
dependency on kudu_util establishes a dependency chain that looks like this:

  version_defines.h
  -> kudu_util
  -> protoc-gen-krpc
  -> <every krpc-related .pb.cc and .pb.h file>
  -> <a lot of other Kudu files>

This explains why whenever version_defines.h is regenerated, we end up
rebuilding so much of Kudu.

So I looked into severing that dependency, and it turns out to be quite
simple. We use kudu_util in protoc-gen-krpc for two things: Status, and
string casing. Both are used just once with reasonable alternatives.

After removing the kudu_util -> protoc-gen-krpc dependency, a "clean" to
"dirty" working tree transition led to a 377-step ninja build with only four
compilation steps (for version_info.cc twice, master-test.cc, and
precompiled.ll.cc). Before this change, it was a 634-step ninja build with
248 compilation steps.

Along the way I did some clean up and modernized a few things.

Change-Id: Ifcde701648ca87c64dd3a09468df97654d58317a
---
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/protoc-gen-krpc.cc
2 files changed, 108 insertions(+), 137 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/14942/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14942
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifcde701648ca87c64dd3a09468df97654d58317a
Gerrit-Change-Number: 14942
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>