Skip to content

Commit

Permalink
makefile: rework the dependency graph for faster test runs startup (#174
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
isaac-io committed Oct 3, 2022
1 parent fb0127d commit cb4e649
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 138 deletions.
166 changes: 84 additions & 82 deletions Makefile
Expand Up @@ -213,9 +213,22 @@ AM_SHARE = $(AM_V_CCLD) $(CXX) $(PLATFORM_SHARED_LDFLAGS)$@ -L. $(patsubst lib%.
# 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)"; \
Expand All @@ -227,7 +240,11 @@ dummy := $(shell (export ROCKSDB_ROOT="$(CURDIR)"; \
export USE_CLANG="$(USE_CLANG)"; \
export LIB_MODE="$(LIB_MODE)"; \
"$(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

Expand Down Expand Up @@ -628,11 +645,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))
Expand Down Expand Up @@ -729,10 +748,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
Expand Down Expand Up @@ -880,29 +895,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,
Expand Down Expand Up @@ -955,15 +967,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)' \
Expand All @@ -972,29 +982,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.
Expand All @@ -1010,16 +1032,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
ifeq ($(filter -DROCKSDB_LITE,$(OPT)),)
Expand All @@ -1030,9 +1044,9 @@ endif
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
Expand Down Expand Up @@ -1113,20 +1127,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 \
Expand Down Expand Up @@ -1962,18 +1963,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

#-------------------------------------------------

Expand Down Expand Up @@ -2316,7 +2318,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)
Expand Down Expand Up @@ -2397,19 +2399,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

Expand All @@ -2430,12 +2432,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 '$@'
Expand All @@ -2444,11 +2446,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 '$@'

Expand Down

0 comments on commit cb4e649

Please sign in to comment.