IMPALA-9373: Trial run of include-what-you-use

Implemented recommendations from IWYU in a subset of
files, mostly in util. Did a few cleanups related to
systematic problems that I noticed as a result.

I noticed that uid-util.h was pulling in boost UUID headers
to a lot of compilation units, so refactored that a little
bit, including pulling out the hash functions into
unique-id-hash.h and moving some inline functions into
client-request-state-map.cc.

Systematically replaced the general boost mutex header with the
internal pthread-based one. This is equivalent for us, since
we assume that boost::mutex is implemented by pthread_mutex_t,
e.g. for the implementation of ConditionVariable.

Switch include guards to pragma once just as general cleanup.

Prefix string with std:: consistently in headers so that they
don't depend on "using" declarations pulled in from random
headers.

Look at includes of C++ stream headers, including iostream and
stringstream, and replaced them with iosfwd or removed them
if possible.

Compile time:
Measured a full ASAN build of the impalad binary on an 8 core
machine with cccache enabled, but cleared. It used very slightly
less CPU, probably because we are still pulling in most of the
same system headers.

Before:
real    9m27.502s
user    64m39.775s
sys     2m49.002s

After:
real    9m26.561s
user    64m28.948s
sys     2m48.252s

So for the moment, the only significant wins are on incremental
builds, where touching header files should not require as many
recompilations. Compile times should start to drop meaningfully
once we thin out more unnecessary includes - currently it seems
like most compile units end up with large chunks of boost/std
code included via transitive header dependencies.

Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20
Reviewed-on: http://gerrit.cloudera.org:8080/15202
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>

Author: timarmstrong

be/src/benchmarks/lock-benchmark.cc

@@ -15,13 +15,13 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <stdlib.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <iostream>
-#include <vector>
 #include <sstream>
+#include <vector>
 #include <boost/thread.hpp>
-#include <boost/thread/mutex.hpp>
+#include <boost/thread/pthread/mutex.hpp>
 #include "util/benchmark.h"
 #include "util/cpu-info.h"
 #include "util/spinlock.h"

be/src/benchmarks/process-wide-locks-benchmark.cc

@@ -38,6 +38,7 @@
 #include "util/stopwatch.h"
 #include "util/thread.h"
 #include "util/uid-util.h"
+#include "util/unique-id-hash.h"
 
 #include "common/init.h"
 #include "common/names.h"

be/src/catalog/catalog-server.h

@@ -15,13 +15,12 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#ifndef IMPALA_CATALOG_CATALOG_SERVER_H
-#define IMPALA_CATALOG_CATALOG_SERVER_H
+#pragma once
 
 #include <string>
 #include <vector>
 #include <boost/shared_ptr.hpp>
-#include <boost/thread/mutex.hpp>
+#include <boost/thread/pthread/mutex.hpp>
 #include <boost/unordered_set.hpp>
 
 #include "gen-cpp/CatalogService.h"
@@ -236,5 +235,3 @@ class CatalogServer {
 };
 
 }
-
-#endif

be/src/codegen/llvm-codegen.cc

@@ -23,7 +23,7 @@
 #include <unordered_set>
 
 #include <boost/algorithm/string.hpp>
-#include <boost/thread/mutex.hpp>
+#include <boost/thread/pthread/mutex.hpp>
 #include <gutil/strings/substitute.h>
 
 #include <llvm/ADT/Triple.h>

be/src/common/logging.cc

@@ -17,19 +17,19 @@
 
 #include "common/logging.h"
 
-#include <boost/thread/locks.hpp>
-#include <boost/thread/mutex.hpp>
-#include <boost/uuid/uuid.hpp>
-#include <boost/uuid/uuid_generators.hpp>
-#include <boost/uuid/uuid_io.hpp>
+#include <stdio.h>
 #include <cerrno>
 #include <ctime>
 #include <fstream>
-#include <gutil/strings/substitute.h>
 #include <iostream>
 #include <map>
 #include <sstream>
-#include <stdio.h>
+#include <boost/thread/locks.hpp>
+#include <boost/thread/pthread/mutex.hpp>
+#include <boost/uuid/uuid.hpp>
+#include <boost/uuid/uuid_generators.hpp>
+#include <boost/uuid/uuid_io.hpp>
+#include <gutil/strings/substitute.h>
 
 #include "common/logging.h"
 #include "service/impala-server.h"

be/src/common/names.h

@@ -136,7 +136,7 @@ using boost::thread;
 using boost::thread_group;
 #endif
 
-#ifdef BOOST_THREAD_MUTEX_HPP
+#ifdef BOOST_THREAD_PTHREAD_MUTEX_HPP
 using boost::mutex;
 using boost::timed_mutex;
 using boost::try_mutex;

be/src/common/object-pool.h

@@ -15,13 +15,11 @@
 // specific language governing permissions and limitations
 // under the License.
 
-
-#ifndef IMPALA_COMMON_OBJECT_POOL_H
-#define IMPALA_COMMON_OBJECT_POOL_H
+#pragma once
 
 #include <vector>
 #include <boost/thread/locks.hpp>
-#include <boost/thread/mutex.hpp>
+#include <boost/thread/pthread/mutex.hpp>
 
 #include "gutil/macros.h"
 #include "util/spinlock.h"
@@ -67,5 +65,3 @@ class ObjectPool {
 };
 
 }
-
-#endif

be/src/exec/blocking-plan-root-sink.cc

@@ -24,7 +24,7 @@
 #include "util/pretty-printer.h"
 
 #include <memory>
-#include <boost/thread/mutex.hpp>
+#include <boost/thread/pthread/mutex.hpp>
 
 using namespace std;
 using boost::unique_lock;

be/src/exec/data-sink.h

@@ -98,8 +98,8 @@ class DataSink {
   /// If this is the sink at the root of a fragment, 'sink_id' must be a unique ID for
   /// the sink for use in runtime profiles and other purposes. Otherwise this is a join
   /// build sink owned by an ExecNode and 'sink_id' must be -1.
-  DataSink(TDataSinkId sink_id, const DataSinkConfig& sink_config, const string& name,
-      RuntimeState* state);
+  DataSink(TDataSinkId sink_id, const DataSinkConfig& sink_config,
+      const std::string& name, RuntimeState* state);
   virtual ~DataSink();
 
   /// Setup. Call before Send(), Open(), or Close() during the prepare phase of the query
@@ -151,7 +151,7 @@ class DataSink {
   const RowDescriptor* row_desc_;
 
   /// The name to be used in profiles etc. Passed by derived classes in the ctor.
-  const string name_;
+  const std::string name_;
 
   /// The runtime profile for this DataSink. Initialized in ctor. Not owned.
   RuntimeProfile* profile_ = nullptr;

be/src/exec/exec-node.cc

@@ -65,6 +65,7 @@
 #include "runtime/runtime-state.h"
 #include "util/debug-util.h"
 #include "util/runtime-profile-counters.h"
+#include "util/uid-util.h"
 
 #include "common/names.h"