Maxim Smyatkin has posted comments on this change. Change subject: KUDU-1696. Daemons should dump their version info to the INFO log at startup ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/util/flags-test.cc File src/kudu/util/flags-test.cc: PS3, Line 90: ndefault > Is this a typo or did you intentionally put an invalid flagfile with 'ndefa typo, thanks http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/util/flags.cc File src/kudu/util/flags.cc: PS3, Line 366: // This only means, that the flag have been rewritten. Doesn't mean, that : // it is done in command line or even that it's trully different from : // default value. > some grammar/spelling mistakes: Ok, thanks for rewriting :) Line 370: for (const auto& default_flag : default_flags) { > rather than the O(n^2) loop here, I think it would be better to capture the I was thinking about this, but it's done only once at startup anyways and if we replace it with a map construction, we will just trade this nested loop for: 1) creating a map (still having the temporary vector) 2) inserting into map, i.e. allocating space for flags, making copies and looking for the right spot with O(N*logN) 3) looking them up in map, yet another N*logN. It's probably not worth doing so unless we have several hundreds of flags. Still want me to change it? -- To view, visit http://gerrit.cloudera.org:8080/4733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I846827c53e74ca364af61c690bf85af8a8f28601 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim Smyatkin <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Maxim Smyatkin <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
