Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12194 )
Change subject: Eliminate redundant VLOG_IS_ON calls ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12194/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12194/1//COMMIT_MSG@13 PS1, Line 13: The expression on the righthand side of the << operator is evaluated : only when VLOG_IS_ON(3) is true. > Sure, feel free to add that and modify to whatever taste you like. Also I And some more context if it makes sense. I started with looking at https://github.com/apache/kudu/blob/920c21aa123feb04b40df4d310f469678d26a8c4/src/kudu/util/thread.cc#L624 because of reports that Thread::StartThread() took relatively long time (~2.5ms), but in the same time there was no report on slow thread creation that would originate from https://github.com/apache/kudu/blob/920c21aa123feb04b40df4d310f469678d26a8c4/src/kudu/util/thread.cc#L599 I suspected that Thread::id() call took long time and so total was 2.5ms, but after looking at how VLOG() works and running a couple of tests it seems that was not the case. Maybe, some other compilers (not clang from LLVM6) are prone to evaluating the right part even if VLOG level is less than required, but at least now we compile with clang from LLVM6 and the generated code for 'VLOG(1) << xxx' looks like following: !(__extension__ ({ static google::int32* vlocal__ = &google::kLogSiteUninitialized; google::int32 verbose_level__ = (1); (*vlocal__ >= verbose_level__) && ((vlocal__ != &google::kLogSiteUninitialized) || (google::InitVLOG3__(&vlocal__, &FLAGS_v, "client-test.cc", verbose_level__))); })) ? (void) 0 : google::LogMessageVoidify() & google::LogMessage( "client-test.cc", 5828).stream() << string("VLOG(1) ") + callNumberToStr(); -- To view, visit http://gerrit.cloudera.org:8080/12194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0781e64f95e33fe067f9f3e65e77aec8654f4ba3 Gerrit-Change-Number: 12194 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 09 Jan 2019 17:14:23 +0000 Gerrit-HasComments: Yes
