Improve portability

While trying to build RAMCloud on a new system, I made various
portability improvements:
- Fixed build and doc issues for Infiniband-less builds.
- Added new private make include at the top of the Makefile for
  overriding default values of make variables.
- Improved error message on failing hash table assertion.
- Bumped up timing threshold in some dubious CyclesTest to get it to
  pass.
- Bumped up message sizes on some dubious TcpTransport tests to get them
  to pass.
- Added preprocessor guards to program_options features that were
  introduced in Boost 1.42. (The project now builds with Boost 1.41.)
- Changed LargeBlockOfMemory to not pin memory. Instead, all memory is
  pinned in the main files of servers now. Most systems only allow users
  to pin 64K of memory by default, so this will fail on many systems. By
  doing it all up front, it's easier to deal with the warnings. Another
  advantage is that all memory is now pinned, not just the large blocks.

Author: Diego Ongaro

Doxyfile

@@ -1302,7 +1302,7 @@ INCLUDE_FILE_PATTERNS  =
 # undefined via #undef or recursively expanded use the := operator
 # instead of the = operator.
 
-PREDEFINED             = DOXYGEN __cplusplus \
+PREDEFINED             = DOXYGEN __cplusplus INFINIBAND \
                          PRIVATE=private PROTECTED=protected PUBLIC=public
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then

GNUmakefile

@@ -4,6 +4,11 @@
 #       Recursive Make Considered Harmful
 #       http://aegis.sourceforge.net/auug97.pdf
 
+
+# The following line allows developers to change the default values for make
+# variables in the file private/MakefragPrivateTop.
+include $(wildcard private/MakefragPrivateTop)
+
 DEBUG ?= yes
 YIELD ?= no
 SSE ?= sse4.2
@@ -69,23 +74,31 @@ COMFLAGS += -DYIELD=1
 endif
 
 CFLAGS_BASE := $(COMFLAGS) -std=gnu0x $(INCLUDES)
+CFLAGS_SILENT := $(CFLAGS_BASE)
 CFLAGS_NOWERROR := $(CFLAGS_BASE) $(CWARNS)
 CFLAGS := $(CFLAGS_BASE) -Werror $(CWARNS)
 
-CXXFLAGS_BASE := $(COMFLAGS) -std=c++0x $(INCLUDES) $(EXTRACXXFLAGS)
-CXXFLAGS_NOWERROR := $(CXXFLAGS_BASE) $(CXXWARNS)
-CXXFLAGS := $(CXXFLAGS_BASE) -Werror $(CXXWARNS)
+CXXFLAGS_BASE := $(COMFLAGS) -std=c++0x $(INCLUDES)
+CXXFLAGS_SILENT := $(CXXFLAGS_BASE) $(EXTRACXXFLAGS)
+CXXFLAGS_NOWERROR := $(CXXFLAGS_BASE) $(CXXWARNS) $(EXTRACXXFLAGS)
+CXXFLAGS := $(CXXFLAGS_BASE) -Werror $(CXXWARNS) $(EXTRACXXFLAGS)
+
 ifeq ($(COMPILER),intel)
 CXXFLAGS = $(CXXFLAGS_BASE) $(CXXWARNS)
 endif
 
-CC := gcc
-CXX := g++
-AR := ar
-PERL := perl
-LINT := python cpplint.py --filter=-runtime/threadsafe_fn,-readability/streams,-whitespace/blank_line,-whitespace/braces,-whitespace/comments,-runtime/arrays,-build/include_what_you_use,-whitespace/semicolon
+CC ?= gcc
+CXX ?= g++
+AR ?= ar
+PERL ?= perl
+PYTHON ?= python
+LINT := $(PYTHON) cpplint.py --filter=-runtime/threadsafe_fn,-readability/streams,-whitespace/blank_line,-whitespace/braces,-whitespace/comments,-runtime/arrays,-build/include_what_you_use,-whitespace/semicolon
 PRAGMAS := ./pragmas.py
 NULL := # useful for terminating lists of files
+PROTOC ?= protoc
+EPYDOC ?= epydoc
+EPYDOCFLAGS ?= --simple-term -v
+DOXYGEN ?= doxygen
 
 # run-cc:
 # Compile a C source file to an object file.
@@ -97,8 +110,8 @@ define run-cc
 @GCCWARN=$$( $(PRAGMAS) -q GCCWARN $(2) ); \
 case $$GCCWARN in \
 0) \
-	echo $(CC) $(CFLAGS_BASE) $(3) -c -o $(1) $(2); \
-	$(CC) $(CFLAGS_BASE) $(3) -c -o $(1) $(2); \
+	echo $(CC) $(CFLAGS_SILENT) $(3) -c -o $(1) $(2); \
+	$(CC) $(CFLAGS_SILENT) $(3) -c -o $(1) $(2); \
 	;; \
 5) \
 	echo $(CC) $(CFLAGS_NOWERROR) $(3) -c -o $(1) $(2); \
@@ -121,8 +134,8 @@ define run-cxx
 @GCCWARN=$$( $(PRAGMAS) -q GCCWARN $(2) ); \
 case $$GCCWARN in \
 0) \
-	echo $(CXX) $(CXXFLAGS_BASE) $(3) -c -o $(1) $(2); \
-	$(CXX) $(CXXFLAGS_BASE) $(3) -c -o $(1) $(2); \
+	echo $(CXX) $(CXXFLAGS_SILENT) $(3) -c -o $(1) $(2); \
+	$(CXX) $(CXXFLAGS_SILENT) $(3) -c -o $(1) $(2); \
 	;; \
 5) \
 	echo $(CXX) $(CXXFLAGS_NOWERROR) $(3) -c -o $(1) $(2); \
@@ -187,7 +200,7 @@ docs: python-docs
 	DOCSID=$$DOCSID-`cat ".git/$$( git symbolic-ref HEAD )" | cut -c1-6` ;\
 	(echo "PROJECT_NUMBER = \"Version [$$DOCSID]\""; \
 	 echo "INPUT = src bindings README $(OBJDIR)"; \
-	 echo "INCLUDE_PATH = $(OBJDIR)"; ) | cat Doxyfile - | doxygen -
+	 echo "INCLUDE_PATH = $(OBJDIR)"; ) | cat Doxyfile - | $(DOXYGEN) -
 
 docs-clean: python-docs-clean
 	rm -rf docs/doxygen/

bindings/python/Makefrag

@@ -1,5 +1,5 @@
 python-docs: $(OBJDIR)/libramcloud.so
-	LD_LIBRARY_PATH=$(OBJDIR):$$LD_LIBRARY_PATH epydoc --html -n RAMCloud -o docs/epydoc/ --simple-term -v bindings/python/*.py
+	LD_LIBRARY_PATH=$(OBJDIR):$$LD_LIBRARY_PATH $(EPYDOC) --html -n RAMCloud -o docs/epydoc/ $(EPYDOCFLAGS) bindings/python/*.py
 
 python-docs-clean:
 	rm -rf docs/epydoc/
@@ -10,7 +10,7 @@ python-test: $(OBJDIR)/libramcloud.so
 		if [ -x $$test ]; then \
 			echo "Running $$test:"; \
 			LD_LIBRARY_PATH=$(OBJDIR):$$LD_LIBRARY_PATH \
-				python $$test -q || failed=1; \
+				$(PYTHON) $$test -q || failed=1; \
 			echo; \
 		fi \
 	done; \

src/Common.cc

@@ -24,6 +24,7 @@
 #include <fcntl.h>
 #include <pcrecpp.h>
 #include <stdarg.h>
+#include <sys/mman.h>
 #include <sys/stat.h>
 
 #include "Common.h"
@@ -301,4 +302,19 @@ string demangle(const char* name) {
     return ret;
 }
 
+/**
+ * Pin all current and future memory pages in memory so that the OS does not
+ * swap them to disk. All RAMCloud server main files should call this.
+ */
+void pinAllMemory() {
+    int r = mlockall(MCL_CURRENT | MCL_FUTURE);
+    if (r != 0) {
+        LOG(WARNING, "Could not lock all memory pages (%s), so the OS might "
+                     "swap memory later. Check your user's \"ulimit -l\" and "
+                     "adjust /etc/security/limits.conf as necessary.",
+                     strerror(errno));
+    }
+}
+
+
 } // namespace RAMCloud

src/Common.h

@@ -580,5 +580,7 @@ prefetch(const T* object)
     prefetch(object, sizeof(*object));
 }
 
+void pinAllMemory();
+
 } // end RAMCloud
 #endif // RAMCLOUD_COMMON_H

src/CoordinatorMain.cc

@@ -33,6 +33,7 @@ main(int argc, char *argv[])
     try {
         OptionParser optionParser(OptionsDescription("Coordinator"),
                                   argc, argv);
+        pinAllMemory();
         string localLocator = optionParser.options.getCoordinatorLocator();
         transportManager.initialize(localLocator.c_str());
         localLocator = transportManager.getListeningLocatorsString();

src/Cperf.cc

@@ -20,6 +20,7 @@
 // benchmark code.
 
 #include <boost/program_options.hpp>
+#include <boost/version.hpp>
 #include <iostream>
 namespace po = boost::program_options;
 
@@ -686,8 +687,12 @@ try
         ("clientIndex", po::value<int>(&clientIndex)->default_value(0),
                 "Index of this client (first client is 0)")
         ("coordinator,C",
+#if BOOST_VERSION >= 104200 // required flag introduced in Boost 1.42
                 po::value<string>(&coordinatorLocator)->required(),
-                "Service locator for the cluster coordinator")
+#else
+                po::value<string>(&coordinatorLocator),
+#endif
+                "Service locator for the cluster coordinator (required)")
         ("help,h", "Print this help message")
         ("numClients", po::value<int>(&numClients)->default_value(1),
                 "Total number of clients running")
@@ -698,7 +703,7 @@ try
     po::variables_map vm;
     po::store(po::command_line_parser(argc, argv).
             options(desc).positional(desc2).run(), vm);
-    if (vm.count("help")) {
+    if (vm.count("help") || coordinatorLocator.empty()) {
         std::cout << desc << '\n';
         exit(0);
     }

src/CyclesTest.cc

@@ -42,12 +42,11 @@ TEST_F(CyclesTest, basics) {
         start = Cycles::rdtsc();
         usleep(1000);
         elapsed = Cycles::toSeconds(Cycles::rdtsc() - start);
-        if ((elapsed >= .001) && (elapsed <.0015)) {
-            break;
-        }
+        ASSERT_LE(.001, elapsed);
+        ASSERT_GT(.0035, elapsed)
+          << "Your system slept longer than it should have. "
+             "There's probably nothing wrong -- this test is dubious.";
     }
-    EXPECT_LE(.001, elapsed);
-    EXPECT_GT(.0015, elapsed);
 }
 
 TEST_F(CyclesTest, perSecond_sanity) {

src/HashTable.h

@@ -887,8 +887,12 @@ class HashTable {
                 typeField <<= (47 - typeBits);
             }
 
-            if ((ptr  & ~(0x00007fffffffffffUL >> typeBits)) != 0)
-                throw Exception(HERE, "pointer cannot fit! stack addr used?");
+            if ((ptr  & ~(0x00007fffffffffffUL >> typeBits)) != 0) {
+                throw Exception(HERE, format(
+                    "The given pointer (0x%016lx) can't fit in a hash table "
+                    "entry. Maybe it's a stack address? typeBits=%u",
+                    ptr, typeBits));
+            }
 
             uint64_t c = chain ? 1 : 0;
             assert((hash & ~(0x000000000000ffffUL)) == 0);

src/LargeBlockOfMemory.h

@@ -44,7 +44,7 @@ struct LargeBlockOfMemory {
         : length(length)
         , block(static_cast<T*>(mmap(NULL, length, PROT_READ | PROT_WRITE,
                                      MAP_SHARED | MAP_ANONYMOUS |
-                                     MAP_LOCKED | MAP_POPULATE, -1, 0)))
+                                     MAP_POPULATE, -1, 0)))
     {
         if (block == MAP_FAILED) {
             if (length == 0)
@@ -101,7 +101,7 @@ struct LargeBlockOfMemory {
         }
 
         block = reinterpret_cast<T*>(mmap(NULL, length, PROT_READ | PROT_WRITE,
-                                         MAP_SHARED | MAP_LOCKED | MAP_POPULATE,
+                                         MAP_SHARED | MAP_POPULATE,
                                          fd, 0));
         if (reinterpret_cast<void*>(block) == MAP_FAILED) {
             unlink(path);

src/Makefrag

@@ -5,9 +5,9 @@ AUTO_GEN_HEADERS += $(OBJDIR)/Metrics.in.h
 .SECONDARY: $(AUTO_GEN_HEADERS)
 $(OBJDIR)/%.pb.cc $(OBJDIR)/%.pb.h: $(TOP)/src/%.proto
 	@mkdir -p $(OBJDIR)
-	@echo protoc ... $$(basename $<)
+	@echo $(PROTOC) ... $$(basename $<)
 	@cd $(TOP)/src/; \
-	protoc --cpp_out=$(TOP)/$(OBJDIR) $$(basename $<); \
+	$(PROTOC) --cpp_out=$(TOP)/$(OBJDIR) $$(basename $<) || exit 1; \
 	echo "// RAMCloud pragma [GCCWARN=0]" >> $(TOP)/$(OBJDIR)/$$(basename $< .proto).pb.h; \
 	echo "// RAMCloud pragma [GCCWARN=0]" >> $(TOP)/$(OBJDIR)/$$(basename $< .proto).pb.cc
 

src/OptionParser.cc

@@ -13,6 +13,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <boost/version.hpp>
 #include <boost/program_options.hpp>
 #include <boost/lexical_cast.hpp>
 #include <iostream>
@@ -179,8 +180,12 @@ OptionParser::setup(int argc, char* argv[])
     catch (po::multiple_occurrences& e) {
         // This clause could provides a more understandable error message
         // (the default is fairly opaque).
+#if BOOST_VERSION >= 104200 // get_option_name introduced in Boost 1.4.2
         throw po::error(format("command-line option '%s' occurs multiple times",
                 e.get_option_name().c_str()));
+#else
+        throw po::error("command-line option occurs multiple times");
+#endif
     }
 }
 

src/ServerMain.cc

@@ -114,6 +114,7 @@ try
         LOG(DEBUG, "server: Pinned to core %d", cpu);
     }
 
+    pinAllMemory();
     transportManager.initialize(
             optionParser.options.getLocalLocator().c_str());
     LOG(NOTICE, "%s: Listening on %s", servicesInfo,

src/TcpTransportTest.cc

@@ -472,15 +472,19 @@ TEST_F(TcpTransportTest, sendMessage_largeBuffer) {
             &reply);
     Transport::ServerRpc* serverRpc = serviceManager->waitForRpc(1.0);
     EXPECT_TRUE(serverRpc != NULL);
-    EXPECT_GT(TcpTransport::messageChunks, 0);
+    EXPECT_GT(TcpTransport::messageChunks, 0)
+        << "The message fit in one chunk. You may have to increase the size "
+           "of the message for this test to be effective.";
     EXPECT_EQ("ok", TestUtil::checkLargeBuffer(&serverRpc->requestPayload,
             300000));
-    TestUtil::fillLargeBuffer(&serverRpc->replyPayload, 250000);
+    TestUtil::fillLargeBuffer(&serverRpc->replyPayload, 350000);
     TcpTransport::messageChunks = 0;
     serverRpc->sendReply();
     EXPECT_TRUE(TestUtil::waitForRpc(*clientRpc));
-    EXPECT_EQ("ok", TestUtil::checkLargeBuffer(&reply, 250000));
-    EXPECT_GT(TcpTransport::messageChunks, 0);
+    EXPECT_GT(TcpTransport::messageChunks, 0)
+        << "The message fit in one chunk. You may have to increase the size "
+           "of the message for this test to be effective.";
+    EXPECT_EQ("ok", TestUtil::checkLargeBuffer(&reply, 350000));
 }
 
 TEST_F(TcpTransportTest, sendMessage_brokenPipe) {
@@ -1044,7 +1048,7 @@ TEST_F(TcpTransportTest, sendReply) {
     serverRpc->sendReply();
     serverRpc = serviceManager->waitForRpc(1.0);
     EXPECT_TRUE(serverRpc != NULL);
-    TestUtil::fillLargeBuffer(&serverRpc->replyPayload, 199999);
+    TestUtil::fillLargeBuffer(&serverRpc->replyPayload, 1999990);
     serverRpc->sendReply();
     serverRpc = serviceManager->waitForRpc(1.0);
     EXPECT_TRUE(serverRpc != NULL);
@@ -1054,15 +1058,17 @@ TEST_F(TcpTransportTest, sendReply) {
     // Check server state.
     EXPECT_NE(server.sockets.size(), 0U);
     TcpTransport::Socket* socket = server.sockets[server.sockets.size() - 1];
-    EXPECT_EQ(2U, socket->rpcsWaitingToReply.size());
+    EXPECT_EQ(2U, socket->rpcsWaitingToReply.size())
+        << "There are no pending RPCs responses to send. You may have to "
+           "increase the size of the message for this test to be effective.";
     EXPECT_EQ("~TcpServerRpc: deleted", TestLog::get());
     TestLog::reset();
 
     // Make sure the responses eventually get through.
     EXPECT_TRUE(TestUtil::waitForRpc(*clientRpc1));
     EXPECT_EQ("response1/0", TestUtil::toString(&reply1));
     EXPECT_TRUE(TestUtil::waitForRpc(*clientRpc2));
-    EXPECT_EQ("ok", TestUtil::checkLargeBuffer(&reply2, 199999));
+    EXPECT_EQ("ok", TestUtil::checkLargeBuffer(&reply2, 1999990));
     EXPECT_TRUE(TestUtil::waitForRpc(*clientRpc3));
     EXPECT_EQ("response3/0", TestUtil::toString(&reply3));
     EXPECT_EQ("~TcpServerRpc: deleted | ~TcpServerRpc: deleted",