makefile: rework the dependency graph for faster test runs startup (#174

) The Makefile has many places where it needlessly invokes a sub-make instead of declaring the dependencies correctly. Rework the dependency graph so that at least running tests doesn't involve invoking make three times. This also avoids pollution in /dev/shm caused by the needless creation of temporary directories on make restarts and sub-make invocations.
speedb-io · Nov 1, 2023 · 98455bf · 98455bf
1 parent 0d849c1
commit 98455bf
Show file tree

Hide file tree

Showing 2 changed files with 147 additions and 140 deletions.
diff --git a/Makefile b/Makefile
@@ -88,9 +88,22 @@ $(info $$DEBUG_LEVEL is $(DEBUG_LEVEL), $$LIB_MODE is $(LIB_MODE))
 # Only regenerate make_config.mk if it doesn't exists or if we're invoked in a mode
 # that executes target recipes (i.e. not -n or -q)
 ifeq ($(and $(or $(findstring n,$(MAKEFLAGS)),$(findstring q,$(MAKEFLAGS))),$(wildcard make_config.mk)),)
+# Only generate make_config.mk during the main make invocation, not on restarts
+# (restarts are caused by Makefiles being updated during the parsing of the Makefile,
+#  which is exactly what happens when make_config.mk is regenerated and included).
+ifeq ($(MAKE_RESTARTS),)
+# If make_config.mk exists and the make invocation was for a target that doesn't
+# need to regenerate it (because it doesn't build anything), such as `make clean`,
+# don't perform the regeneration since these targets either don't need make_config.mk
+# at all or only need to use the existing configuration in make_config.mk to do
+# their job.
+NO_CONFIG_REGENERATION_TARGETS := clean% jclean uninstall dump-log watch-log tags% format check-format check-buck-targets check-sources package checkout_folly list_all_tests
+ifneq ($(strip $(and $(wildcard make_config.mk),$(filter-out $(NO_CONFIG_REGENERATION_TARGETS),$(MAKECMDGOALS) make_config.mk))),make_config.mk)
+
 # Detect what platform we're building on.
 # Export some common variables that might have been passed as Make variables
 # instead of environment variables.
+$(info * GEN     make_config.mk)
 dummy := $(shell (export ROCKSDB_ROOT="$(CURDIR)"; \
                   export CXXFLAGS="$(EXTRA_CXXFLAGS)"; \
                   export LDFLAGS="$(EXTRA_LDFLAGS)"; \
@@ -104,7 +117,11 @@ dummy := $(shell (export ROCKSDB_ROOT="$(CURDIR)"; \
 		  export ROCKSDB_CXX_STANDARD="$(ROCKSDB_CXX_STANDARD)"; \
 		  export USE_FOLLY="$(USE_FOLLY)"; \
                   "$(CURDIR)/build_tools/build_detect_platform" "$(CURDIR)/make_config.mk"))
+
+endif
 endif
+endif
+
 # this file is generated by the previous line to set build flags and sources
 include make_config.mk
 
@@ -685,11 +702,13 @@ ROCKSDBTESTS_SUBSET ?= $(TESTS)
 #   its various tests. Parallel can fill up your /dev/shm
 # db_bloom_filter_test - serial because excessive space usage by instances
 #   of DBFilterConstructionReserveMemoryTestWithParam can fill up /dev/shm
+# timer_queue_test - doesn't use gtest
 NON_PARALLEL_TEST = \
 	c_test \
 	env_test \
 	deletefile_test \
 	db_bloom_filter_test \
+	timer_queue_test \
 	$(PLUGIN_TESTS) \
 
 PARALLEL_TEST = $(filter-out $(NON_PARALLEL_TEST), $(TESTS))
@@ -784,10 +803,6 @@ TOOLS_LIBRARY=$(STATIC_TOOLS_LIBRARY)
 endif
 STRESS_LIBRARY=$(STATIC_STRESS_LIBRARY)
 
-ROCKSDB_MAJOR = $(shell grep -E "ROCKSDB_MAJOR.[0-9]" include/rocksdb/version.h | cut -d ' ' -f 3)
-ROCKSDB_MINOR = $(shell grep -E "ROCKSDB_MINOR.[0-9]" include/rocksdb/version.h | cut -d ' ' -f 3)
-ROCKSDB_PATCH = $(shell grep -E "ROCKSDB_PATCH.[0-9]" include/rocksdb/version.h | cut -d ' ' -f 3)
-
 # If NO_UPDATE_BUILD_VERSION is set we don't update util/build_version.cc, but
 # the file needs to already exist or else the build will fail
 ifndef NO_UPDATE_BUILD_VERSION
@@ -928,29 +943,26 @@ coverage: clean
 #
 
 parallel_tests = $(patsubst %,parallel_%,$(PARALLEL_TEST))
-.PHONY: gen_parallel_tests $(parallel_tests)
-$(parallel_tests):
-	$(AM_V_at)TEST_BINARY=$(patsubst parallel_%,%,$@); \
+.PHONY: $(parallel_tests)
+$(parallel_tests): $(parallel_tests:parallel_%=%)
+	$(AM_V_at)mkdir -p t; \
+	TEST_BINARY=$(patsubst parallel_%,%,$@); \
   TEST_NAMES=` \
     (./$$TEST_BINARY --gtest_list_tests || echo "  $${TEST_BINARY}__list_tests_failure") \
     | awk '/^[^ ]/ { prefix = $$1 } /^[ ]/ { print prefix $$1 }'`; \
 	echo "  Generating parallel test scripts for $$TEST_BINARY"; \
+	rm -f t/run-$${TEST_BINARY}-*; \
 	for TEST_NAME in $$TEST_NAMES; do \
-		TEST_SCRIPT=t/run-$$TEST_BINARY-$${TEST_NAME//\//-}; \
+		TEST_SCRIPT=run-$${TEST_BINARY}-$${TEST_NAME//\//-}; \
     printf '%s\n' \
       '#!/bin/sh' \
-      "d=\$(TEST_TMPDIR)$$TEST_SCRIPT" \
-      'mkdir -p $$d' \
-      "TEST_TMPDIR=\$$d $(DRIVER) ./$$TEST_BINARY --gtest_filter=$$TEST_NAME" \
-		> $$TEST_SCRIPT; \
-		chmod a=rx $$TEST_SCRIPT; \
+      "d=\"$(TEST_TMPDIR)/$$TEST_SCRIPT\"" \
+      'mkdir -p "$$d"' \
+      "TEST_TMPDIR=\"\$$d\" $(DRIVER) ./$$TEST_BINARY --gtest_filter=$$TEST_NAME && rm -rf \"\$$d\"" \
+		> t/$$TEST_SCRIPT; \
+		chmod a=rx t/$$TEST_SCRIPT; \
 	done
 
-gen_parallel_tests:
-	$(AM_V_at)mkdir -p t
-	$(AM_V_at)$(FIND) t -type f -name 'run-*' -exec rm -f {} \;
-	$(MAKE) $(parallel_tests)
-
 # Reorder input lines (which are one per test) so that the
 # longest-running tests appear first in the output.
 # Do this by prefixing each selected name with its duration,
@@ -1003,15 +1015,13 @@ else
 	parallel_redir = >& t/$(test_log_prefix)log-{/} || bash -c "cat t/$(test_log_prefix)log-{/}; exit $$?"
 endif
 
-.PHONY: check_0
-check_0: gen_parallel_tests
+.PHONY: check_0 check_1
+check_0: $(TESTS) $(parallel_tests)
 	$(AM_V_GEN)printf '%s\n' ''						\
+	  'Running tests in $(TEST_TMPDIR)'		\
 	  'To monitor subtest <duration,pass/fail,name>,'		\
 	  '  run "make watch-log" in a separate window' '';		\
-	{ \
-		printf './%s\n' $(filter-out $(PARALLEL_TEST),$(TESTS)); \
-		find t -name 'run-*' -print; \
-	} \
+		printf './%s\n' $(filter-out $(PARALLEL_TEST),$(TESTS)) $(PARALLEL_TEST:%=t/run-%-*) \
 	  | $(prioritize_long_running_tests)				\
 	  | grep -E '$(tests-regexp)'					\
 	  | grep -E -v '$(EXCLUDE_TESTS_REGEX)'					\
@@ -1020,29 +1030,41 @@ check_0: gen_parallel_tests
 	parallel_retcode=$$? ; \
 	awk '{ if ($$7 != 0 || $$8 != 0) { if ($$7 == "Exitval") { h = $$0; } else { if (!f) print h; print; f = 1 } } } END { if(f) exit 1; }' < LOG ; \
 	awk_retcode=$$?; \
-	if [ $$parallel_retcode -ne 0 ] || [ $$awk_retcode -ne 0 ] ; then exit 1 ; fi
+	if [ $$parallel_retcode -ne 0 ] || [ $$awk_retcode -ne 0 ] ; then exit 1 ; fi;
+
+check_1: $(TESTS)
+	$(AM_V_GEN)for t in $(TESTS); do                          \
+	      echo "===== Running $$t (`date`)"; ./$$t || exit 1; \
+	done;
 
 valgrind-exclude-regexp = InlineSkipTest.ConcurrentInsert|TransactionStressTest.DeadlockStress|DBCompactionTest.SuggestCompactRangeNoTwoLevel0Compactions|BackupableDBTest.RateLimiting|DBTest.CloseSpeedup|DBTest.ThreadStatusFlush|DBTest.RateLimitingTest|DBTest.EncodeDecompressedBlockSizeTest|FaultInjectionTest.UninstalledCompaction|HarnessTest.Randomized|ExternalSSTFileTest.CompactDuringAddFileRandom|ExternalSSTFileTest.IngestFileWithGlobalSeqnoRandomized|MySQLStyleTransactionTest.TransactionStressTest
 
-.PHONY: valgrind_check_0
+.PHONY: valgrind_check_0 valgrind_check_1
 valgrind_check_0: test_log_prefix := valgrind_
-valgrind_check_0: gen_parallel_tests
+valgrind_check_0: $(TESTS) $(parallel_tests)
 	$(AM_V_GEN)printf '%s\n' ''						\
+	  'Running tests in $(TEST_TMPDIR)'		\
 	  'To monitor subtest <duration,pass/fail,name>,'		\
 	  '  run "make watch-log" in a separate window' '';		\
-	{								\
-	  printf './%s\n' $(filter-out $(PARALLEL_TEST) %skiplist_test options_settable_test, $(TESTS));		\
-	  find t -name 'run-*' -print; \
-	}								\
+	  printf './%s\n' $(filter-out $(PARALLEL_TEST) %skiplist_test options_settable_test, $(TESTS)) $(PARALLEL_TEST:%=t/run-%-*) \
 	  | $(prioritize_long_running_tests)				\
 	  | grep -E '$(tests-regexp)'					\
 	  | grep -E -v '$(valgrind-exclude-regexp)'					\
 	  | "$(PARALLEL)" -j$(J) --plain --joblog=LOG --eta --gnu \
 	   --tmpdir=$(TEST_TMPDIR) --timeout=$(TEST_TIMEOUT) \
-	   '(if [[ "{}" == "./"* ]] ; then $(DRIVER) {}; else {}; fi) \
+	   '(if [[ "{}" == "./"* ]] ; then $(VALGRIND_VER) $(VALGRIND_OPTS) {}; else {}; fi) \
 	  $(parallel_redir)' \
 
-CLEAN_FILES += t LOG $(TEST_TMPDIR)
+valgrind_check_1: $(TESTS)
+	$(AM_V_GEN)for t in $(filter-out %skiplist_test options_settable_test,$(TESTS)); do \
+			$(VALGRIND_VER) $(VALGRIND_OPTS) ./$$t; \
+			ret_code=$$?; \
+			if [ $$ret_code -ne 0 ]; then \
+				exit $$ret_code; \
+			fi; \
+		done;
+
+CLEAN_FILES += t LOG
 
 # When running parallel "make check", you can monitor its progress
 # from another window.
@@ -1058,16 +1080,8 @@ dump-log:
 	tail -n+2 LOG|$(parallel_log_extract)
 
 # If J != 1 and GNU parallel is installed, run the tests in parallel,
-# via the check_0 rule above.  Otherwise, run them sequentially.
-check: all
-	$(AM_V_GEN)if [ "$(J)" != "1" ] && [ "$(PARALLEL_OK)" = "1" ];  \
-	then                                                            \
-	    $(MAKE) T="$$t" check_0;                                    \
-	else                                                            \
-	    for t in $(TESTS); do                                       \
-	      echo "===== Running $$t (`date`)"; ./$$t || exit 1; done;          \
-	fi
-	rm -rf $(TEST_TMPDIR)
+# via the check_0 rule above.  Otherwise, run them sequentially via check_1.
+check: all $(if $(shell [ "$(J)" != "1" ] && [ "$(PARALLEL_OK)" = "1" ] && echo 1),check_0,check_1)
 ifneq ($(PLATFORM), OS_AIX)
 	$(PYTHON) tools/check_all_python.py
 ifndef ASSERT_STATUS_CHECKED # not yet working with these tests
@@ -1076,9 +1090,9 @@ ifndef ASSERT_STATUS_CHECKED # not yet working with these tests
 endif
 endif
 ifndef SKIP_FORMAT_BUCK_CHECKS
-	$(MAKE) check-format
-	$(MAKE) check-buck-targets
-	$(MAKE) check-sources
+	build_tools/format-diff.sh -c
+	buckifier/check_buck_targets.sh
+	build_tools/check-sources.sh
 endif
 
 # TODO add ldb_tests
@@ -1159,20 +1173,7 @@ valgrind_test:
 valgrind_test_some:
 	ROCKSDB_VALGRIND_RUN=1 DISABLE_JEMALLOC=1 $(MAKE) valgrind_check_some
 
-valgrind_check: $(TESTS)
-	$(AM_V_GEN)if [ "$(J)" != "1" ] && [ "$(PARALLEL_OK)" = "1" ];  \
-	then                                                            \
-	  $(MAKE)                                                       \
-	  DRIVER="$(VALGRIND_VER) $(VALGRIND_OPTS)" valgrind_check_0;   \
-	else                                                            \
-		for t in $(filter-out %skiplist_test options_settable_test,$(TESTS)); do \
-			$(VALGRIND_VER) $(VALGRIND_OPTS) ./$$t; \
-			ret_code=$$?; \
-			if [ $$ret_code -ne 0 ]; then \
-				exit $$ret_code; \
-			fi; \
-		done; \
-	fi
+valgrind_check: $(if $(shell [ "$(J)" != "1" ] && [ "$(PARALLEL_OK)" = "1" ] && echo 1),valgrind_check_0,valgrind_check_1)
 
 valgrind_check_some: $(ROCKSDBTESTS_SUBSET)
 	for t in $(ROCKSDBTESTS_SUBSET); do \
@@ -2033,18 +2034,19 @@ install: install-static
 
 # Generate the pkg-config file
 gen-pc:
-	-echo 'prefix=$(PREFIX)' > rocksdb.pc
-	-echo 'exec_prefix=$${prefix}' >> rocksdb.pc
-	-echo 'includedir=$${prefix}/include' >> rocksdb.pc
-	-echo 'libdir=$(LIBDIR)' >> rocksdb.pc
-	-echo '' >> rocksdb.pc
-	-echo 'Name: rocksdb' >> rocksdb.pc
-	-echo 'Description: An embeddable persistent key-value store for fast storage' >> rocksdb.pc
-	-echo Version: $(shell ./build_tools/version.sh full) >> rocksdb.pc
-	-echo 'Libs: -L$${libdir} $(EXEC_LDFLAGS) -lrocksdb' >> rocksdb.pc
-	-echo 'Libs.private: $(PLATFORM_LDFLAGS)' >> rocksdb.pc
-	-echo 'Cflags: -I$${includedir} $(PLATFORM_CXXFLAGS)' >> rocksdb.pc
-	-echo 'Requires: $(subst ",,$(ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES))' >> rocksdb.pc
+	$(AM_V_GEN)printf '%s\n' \
+		'prefix=$(PREFIX)' \
+		'exec_prefix=$${prefix}' \
+		'includedir=$${prefix}/include' \
+		'libdir=$(LIBDIR)' \
+		'' \
+		'Name: rocksdb' \
+		'Description: An embeddable persistent key-value store for fast storage' \
+		'Version: $(shell ./build_tools/version.sh full)' \
+		'Libs: -L$${libdir} $(EXEC_LDFLAGS) -lrocksdb' \
+		'Libs.private: $(PLATFORM_LDFLAGS)' \
+		'Cflags: -I$${includedir} $(PLATFORM_CXXFLAGS)' \
+		'Requires: $(subst ",,$(ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES))' > rocksdb.pc
 
 #-------------------------------------------------
 
@@ -2388,7 +2390,7 @@ rocksdbjavastaticnexusbundlejar: rocksdbjavageneratepom
 
 # A version of each $(LIBOBJECTS) compiled with -fPIC
 
-jl/%.o: %.cc
+jl/%.o: %.cc make_config.mk
 	$(AM_V_CC)mkdir -p $(@D) && $(CXX) $(CXXFLAGS) -fPIC -c $< -o $@ $(COVERAGEFLAGS)
 
 rocksdbjava: $(LIB_OBJECTS)
@@ -2507,19 +2509,19 @@ IOSVERSION=$(shell defaults read $(PLATFORMSROOT)/iPhoneOS.platform/version CFBu
 
 else
 ifeq ($(HAVE_POWER8),1)
-$(OBJ_DIR)/util/crc32c_ppc.o: util/crc32c_ppc.c
+$(OBJ_DIR)/util/crc32c_ppc.o: util/crc32c_ppc.c make_config.mk
 	$(AM_V_CC)$(CC) $(CFLAGS) -c $< -o $@
 
-$(OBJ_DIR)/util/crc32c_ppc_asm.o: util/crc32c_ppc_asm.S
+$(OBJ_DIR)/util/crc32c_ppc_asm.o: util/crc32c_ppc_asm.S make_config.mk
 	$(AM_V_CC)$(CC) $(CFLAGS) -c $< -o $@
 endif
-$(OBJ_DIR)/%.o: %.cc
+$(OBJ_DIR)/%.o: %.cc make_config.mk
 	$(AM_V_CC)mkdir -p $(@D) && $(CXX) $(CXXFLAGS) -c $< -o $@ $(COVERAGEFLAGS)
 
-$(OBJ_DIR)/%.o: %.cpp
+$(OBJ_DIR)/%.o: %.cpp make_config.mk
 	$(AM_V_CC)mkdir -p $(@D) && $(CXX) $(CXXFLAGS) -c $< -o $@ $(COVERAGEFLAGS)
 
-$(OBJ_DIR)/%.o: %.c
+$(OBJ_DIR)/%.o: %.c make_config.mk
 	$(AM_V_CC)$(CC) $(CFLAGS) -c $< -o $@
 endif
 
@@ -2540,12 +2542,12 @@ endif
 
 # The .d file indicates .cc file's dependencies on .h files. We generate such
 # dependency by g++'s -MM option, whose output is a make dependency rule.
-$(OBJ_DIR)/%.cc.d: %.cc
+$(OBJ_DIR)/%.cc.d: %.cc make_config.mk
 	@mkdir -p $(@D) && $(CXX) $(CXXFLAGS) $(PLATFORM_SHARED_CFLAGS) \
 	  -MM -MT'$@' -MT'$(<:.cc=.o)' -MT'$(<:%.cc=$(OBJ_DIR)/%.o)' \
           "$<" -o '$@'
 
-$(OBJ_DIR)/%.cpp.d: %.cpp
+$(OBJ_DIR)/%.cpp.d: %.cpp make_config.mk
 	@mkdir -p $(@D) && $(CXX) $(CXXFLAGS) $(PLATFORM_SHARED_CFLAGS) \
 	  -MM -MT'$@' -MT'$(<:.cpp=.o)' -MT'$(<:%.cpp=$(OBJ_DIR)/%.o)' \
           "$<" -o '$@'
@@ -2554,11 +2556,11 @@ ifeq ($(HAVE_POWER8),1)
 DEPFILES_C = $(patsubst %.c, $(OBJ_DIR)/%.c.d, $(LIB_SOURCES_C))
 DEPFILES_ASM = $(patsubst %.S, $(OBJ_DIR)/%.S.d, $(LIB_SOURCES_ASM))
 
-$(OBJ_DIR)/%.c.d: %.c
+$(OBJ_DIR)/%.c.d: %.c make_config.mk
 	@$(CXX) $(CXXFLAGS) $(PLATFORM_SHARED_CFLAGS) \
 	  -MM -MT'$@' -MT'$(<:.c=.o)' "$<" -o '$@'
 
-$(OBJ_DIR)/%.S.d: %.S
+$(OBJ_DIR)/%.S.d: %.S make_config.mk
 	@$(CXX) $(CXXFLAGS) $(PLATFORM_SHARED_CFLAGS) \
 	  -MM -MT'$@' -MT'$(<:.S=.o)' "$<" -o '$@'