[Asterisk-code-review] Build System: Improve ccache matching for different menusele... (asterisk[master])

Jenkins2 asteriskteam at digium.com
Tue Aug 14 13:41:33 CDT 2018


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/9806 )

Change subject: Build System: Improve ccache matching for different menuselect options.
......................................................................

Build System: Improve ccache matching for different menuselect options.

Changing any Menuselect option in the `Compiler Flags` section causes a
full rebuild of the Asterisk source tree.  Every enabled option causes
a #define to be added to buildopts.h, thus breaking ccache caching for
every source file that includes "asterisk.h".  In most cases each option
only applies to one or two files.  Now we only define those options for
the specific sources which use them, this causes much better cache
matching when working with multiple builds.  For example testing code
with an without MALLOC_DEBUG will now use just over half the ccache
size, only main/astmm.o will have two builds cached instead of every
file.

Reorder main/Makefile so _ASTCFLAGS set on specific object files are all
together, sorted by filename.  Stop adding -DMALLOC_DEBUG to CFLAGS of
bundled pjproject, this define is no longer used by any header so only
serves to break cache.

The only code change is a slight adjustment to how main/astmm.c is
initialized.  Initialization functions always exist so main/asterisk.c
can call them unconditionally.  Additionally rename the astmm
initialization functions so they are not exported.

Change-Id: Ie2085237a964f6e1e6fff55ed046e2afff83c027
---
M Makefile.rules
M build_tools/make_buildopts_h
M channels/Makefile
M include/asterisk/_private.h
M include/asterisk/astmm.h
M main/Makefile
M main/asterisk.c
M main/astmm.c
M tests/Makefile
M third-party/pjproject/Makefile
M utils/Makefile
11 files changed, 75 insertions(+), 52 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Richard Mudgett: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/Makefile.rules b/Makefile.rules
index 7f917b4..b979927 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -20,6 +20,9 @@
 # Helpful functions
 # call with $(call function,...)
 tolower = $(shell echo $(1) | tr '[:upper:]' '[:lower:]')
+# Takes a list of MENUSELECT_CFLAG Id and returns CFLAGS to declare
+# the ones which are enabled.
+get_menuselect_cflags=$(patsubst %,-D%,$(filter $1,$(MENUSELECT_CFLAGS)))
 
 .PHONY: dist-clean
 
diff --git a/build_tools/make_buildopts_h b/build_tools/make_buildopts_h
index 57df27d..692e15b 100755
--- a/build_tools/make_buildopts_h
+++ b/build_tools/make_buildopts_h
@@ -20,21 +20,30 @@
 
 TMP=`${GREP} -e "^MENUSELECT_CFLAGS" menuselect.makeopts | sed 's/MENUSELECT_CFLAGS\=//g' | sed 's/-D//g'`
 for x in ${TMP}; do
-	echo "#define ${x} 1"
 	if test "${x}" = "AO2_DEBUG" \
 			-o "${x}" = "BETTER_BACKTRACES" \
 			-o "${x}" = "BUILD_NATIVE" \
+			-o "${x}" = "COMPILE_DOUBLE" \
+			-o "${x}" = "DEBUG_CHAOS" \
 			-o "${x}" = "DEBUG_SCHEDULER" \
 			-o "${x}" = "DETECT_DEADLOCKS" \
 			-o "${x}" = "DONT_OPTIMIZE" \
 			-o "${x}" = "DUMP_SCHEDULER" \
 			-o "${x}" = "LOTS_OF_SPANS" \
-			-o "${x}" = "LOW_MEMORY" \
 			-o "${x}" = "MALLOC_DEBUG" \
 			-o "${x}" = "RADIO_RELAX" \
 			-o "${x}" = "REBUILD_PARSERS" \
-			-o "${x}" = "REF_DEBUG" ; then
-		# These aren't ABI affecting options, keep them out of AST_BUILDOPTS
+			-o "${x}" = "REF_DEBUG" \
+			-o "${x}" = "USE_HOARD_ALLOCATOR" ; then
+		# These options are only for specific sources and have no effect on public ABI.
+		# Keep them out of buildopts.h so ccache does not invalidate all sources.
+		continue
+	fi
+
+	echo "#define ${x} 1"
+	if test "${x}" = "LOW_MEMORY" ; then
+		# LOW_MEMORY isn't an ABI affecting option but it is used in many sources
+		# so it gets defined globally but is not included in AST_BUILTOPTS.
 		continue
 	fi
 	if test "x${BUILDOPTS}" != "x" ; then
diff --git a/channels/Makefile b/channels/Makefile
index 6398d95..96421d8 100644
--- a/channels/Makefile
+++ b/channels/Makefile
@@ -29,6 +29,7 @@
 $(call MOD_ADD_C,chan_dahdi,$(wildcard dahdi/*.c) sig_analog.c sig_pri.c sig_ss7.c)
 $(call MOD_ADD_C,chan_misdn,misdn_config.c misdn/isdn_lib.c misdn/isdn_msg_parser.c)
 
+chan_dahdi.o: _ASTCFLAGS+=$(call get_menuselect_cflags,LOTS_OF_SPANS)
 chan_mgcp.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
 chan_unistim.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
 chan_phone.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index 89a8f54..d954768 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -58,6 +58,22 @@
 int dns_core_init(void);        /*!< Provided by dns_core.c */
 
 /*!
+ * \brief Initialize malloc debug phase 1.
+ *
+ * \note Must be called first thing after forking.
+ *
+ * \return Nothing
+ */
+void load_astmm_phase_1(void);
+
+/*!
+ * \brief Initialize malloc debug phase 2.
+ *
+ * \return Nothing
+ */
+void load_astmm_phase_2(void);
+
+/*!
  * \brief Initialize the bridging system.
  * \since 12.0.0
  *
diff --git a/include/asterisk/astmm.h b/include/asterisk/astmm.h
index 13ebe03..e1f91dd 100644
--- a/include/asterisk/astmm.h
+++ b/include/asterisk/astmm.h
@@ -31,13 +31,6 @@
 #define _ASTERISK_ASTMM_H
 /* IWYU pragma: private, include "asterisk.h" */
 
-#if defined(MALLOC_DEBUG) && !defined(STANDALONE) && !defined(STANDALONE2)
-#define __AST_DEBUG_MALLOC
-
-void __ast_mm_init_phase_1(void);
-void __ast_mm_init_phase_2(void);
-#endif
-
 void *ast_std_malloc(size_t size) attribute_malloc;
 void *ast_std_calloc(size_t nmemb, size_t size) attribute_malloc;
 void *ast_std_realloc(void *ptr, size_t size);
diff --git a/main/Makefile b/main/Makefile
index 0e29c84..1cb2c25 100644
--- a/main/Makefile
+++ b/main/Makefile
@@ -141,22 +141,34 @@
 	$(CMD_PREFIX) cat $@.fix >> $@
 	$(CMD_PREFIX) rm $@.fix
 
-ast_expr2f.o: _ASTCFLAGS+=-Wno-unused
-cdr.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
-
-db.o: _ASTCFLAGS+=$(SQLITE3_INCLUDE)
-asterisk.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)
-json.o: _ASTCFLAGS+=$(JANSSON_INCLUDE)
-bucket.o: _ASTCFLAGS+=$(URIPARSER_INCLUDE)
-crypt.o: _ASTCFLAGS+=$(CRYPT_INCLUDE)
-uuid.o: _ASTCFLAGS+=$(UUID_INCLUDE)
-
 ifneq ($(findstring ENABLE_UPLOADS,$(MENUSELECT_CFLAGS)),)
-http.o: _ASTCFLAGS+=$(GMIME_INCLUDE)
+GMIMELDFLAGS+=$(GMIME_LIB)
+GMIMECFLAGS+=$(GMIME_INCLUDE)
 endif
 
+# Alter CFLAGS for specific sources
 stdtime/localtime.o: _ASTCFLAGS+=$(AST_NO_STRICT_OVERFLOW) -Wno-format-nonliteral
 
+asterisk.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)
+ast_expr2f.o: _ASTCFLAGS+=-Wno-unused
+astmm.o: _ASTCFLAGS+=$(call get_menuselect_cflags,MALLOC_DEBUG DEBUG_CHAOS)
+astobj2.o astobj2_container.o astobj2_hash.o astobj2_rbtree.o: _ASTCFLAGS+=$(call get_menuselect_cflags,AO2_DEBUG)
+backtrace.o: _ASTCFLAGS+=$(call get_menuselect_cflags,BETTER_BACKTRACES)
+bucket.o: _ASTCFLAGS+=$(URIPARSER_INCLUDE)
+cdr.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
+crypt.o: _ASTCFLAGS+=$(CRYPT_INCLUDE)
+db.o: _ASTCFLAGS+=$(SQLITE3_INCLUDE)
+dsp.o: _ASTCFLAGS+=$(call get_menuselect_cflags,RADIO_RELAX)
+http.o: _ASTCFLAGS+=$(GMIMECFLAGS)
+iostream.o: _ASTCFLAGS+=$(OPENSSL_INCLUDE)
+json.o: _ASTCFLAGS+=$(JANSSON_INCLUDE)
+lock.o: _ASTCFLAGS+=$(call get_menuselect_cflags,DETECT_DEADLOCKS)
+options.o: _ASTCFLAGS+=$(call get_menuselect_cflags,REF_DEBUG)
+sched.o: _ASTCFLAGS+=$(call get_menuselect_cflags,DEBUG_SCHEDULER DUMP_SCHEDULER)
+tcptls.o: _ASTCFLAGS+=$(OPENSSL_INCLUDE) -Wno-deprecated-declarations
+uuid.o: _ASTCFLAGS+=$(UUID_INCLUDE)
+
+
 OBJS:=$(sort $(OBJS))
 
 ifneq ($(findstring $(OSARCH), mingw32 cygwin ),)
@@ -169,10 +181,6 @@
 MAIN_TGT:=asterisk
 endif
 
-ifneq ($(findstring ENABLE_UPLOADS,$(MENUSELECT_CFLAGS)),)
-GMIMELDFLAGS+=$(GMIME_LIB)
-endif
-
 $(OBJS): _ASTCFLAGS+=-DAST_MODULE=\"core\" -DAST_IN_CORE
 $(MOD_OBJS): _ASTCFLAGS+=$(call MOD_ASTCFLAGS,$*)
 
@@ -306,9 +314,6 @@
 
 endif
 
-iostream.o: _ASTCFLAGS+=$(OPENSSL_INCLUDE)
-tcptls.o: _ASTCFLAGS+=$(OPENSSL_INCLUDE) -Wno-deprecated-declarations
-
 $(MAIN_TGT): $(OBJS) $(MOD_OBJS) $(ASTSSL_LIB) $(ASTPJ_LIB)
 	@$(CC) -c -o buildinfo.o $(_ASTCFLAGS) buildinfo.c $(ASTCFLAGS)
 	$(ECHO_PREFIX) echo "   [LD] $(OBJS) $(MOD_OBJS) -> $@"
diff --git a/main/asterisk.c b/main/asterisk.c
index 4548420..1d52567 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -3964,9 +3964,7 @@
 	 * an Asterisk instance, and that there isn't one already running. */
 	multi_thread_safe = 1;
 
-#if defined(__AST_DEBUG_MALLOC)
-	__ast_mm_init_phase_1();
-#endif	/* defined(__AST_DEBUG_MALLOC) */
+	load_astmm_phase_1();
 
 	/* Check whether high prio was succesfully set by us or some
 	 * other incantation. */
@@ -4171,9 +4169,7 @@
 
 	pthread_sigmask(SIG_UNBLOCK, &sigs, NULL);
 
-#if defined(__AST_DEBUG_MALLOC)
-	__ast_mm_init_phase_2();
-#endif	/* defined(__AST_DEBUG_MALLOC) */
+	load_astmm_phase_2();
 
 	ast_cli_register_multiple(cli_asterisk_shutdown, ARRAY_LEN(cli_asterisk_shutdown));
 	ast_cli_register_multiple(cli_asterisk, ARRAY_LEN(cli_asterisk));
diff --git a/main/astmm.c b/main/astmm.c
index 08f67ab..c845ee7 100644
--- a/main/astmm.c
+++ b/main/astmm.c
@@ -31,6 +31,7 @@
 #define ASTMM_LIBC ASTMM_IGNORE
 #include "asterisk.h"
 
+#include "asterisk/_private.h"
 #include "asterisk/logger.h"
 
 /*!
@@ -62,6 +63,10 @@
 #define ast_log_safe ast_log
 #endif
 
+#if defined(MALLOC_DEBUG) && !defined(STANDALONE) && !defined(STANDALONE2)
+#define __AST_DEBUG_MALLOC
+#endif
+
 #define MALLOC_FAILURE_MSG \
 	ast_log_safe(LOG_ERROR, "Memory Allocation Failure in function %s at line %d of %s\n", func, lineno, file)
 
@@ -1501,14 +1506,7 @@
 	}
 }
 
-/*!
- * \brief Initialize malloc debug phase 1.
- *
- * \note Must be called first thing in main().
- *
- * \return Nothing
- */
-void __ast_mm_init_phase_1(void)
+void load_astmm_phase_1(void)
 {
 	atexit(mm_atexit_final);
 }
@@ -1522,12 +1520,7 @@
 	ast_cli_unregister_multiple(cli_memory, ARRAY_LEN(cli_memory));
 }
 
-/*!
- * \brief Initialize malloc debug phase 2.
- *
- * \return Nothing
- */
-void __ast_mm_init_phase_2(void)
+void load_astmm_phase_2(void)
 {
 	char filename[PATH_MAX];
 
@@ -1550,6 +1543,14 @@
 
 #else	/* !defined(__AST_DEBUG_MALLOC) */
 
+void load_astmm_phase_1(void)
+{
+}
+
+void load_astmm_phase_2(void)
+{
+}
+
 void *__ast_repl_calloc(size_t nmemb, size_t size, const char *file, int lineno, const char *func)
 {
 	DEBUG_CHAOS_RETURN(DEBUG_CHAOS_ALLOC_CHANCE, NULL);
diff --git a/tests/Makefile b/tests/Makefile
index f64669b..715c3f8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -19,5 +19,6 @@
 
 include $(ASTTOPDIR)/Makefile.moddir_rules
 
+test_astobj2.o: _ASTCFLAGS+=$(call get_menuselect_cflags,AO2_DEBUG)
 test_strings.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION) $(AST_NO_STRINGOP_TRUNCATION)
 test_voicemail_api.o: _ASTCFLAGS+=$(AST_NO_FORMAT_TRUNCATION)
diff --git a/third-party/pjproject/Makefile b/third-party/pjproject/Makefile
index 476eb44..c761cb5 100644
--- a/third-party/pjproject/Makefile
+++ b/third-party/pjproject/Makefile
@@ -64,9 +64,6 @@
         ifeq ($(AST_DEVMODE),yes)
             CF += -DPJPROJECT_BUNDLED_ASSERTIONS=yes
         endif
-        ifeq ($(findstring MALLOC_DEBUG,$(MENUSELECT_CFLAGS)),MALLOC_DEBUG)
-            CF += -DMALLOC_DEBUG
-        endif
         MALLOC_DEBUG_LIBS = source/pjsip-apps/lib/libasterisk_malloc_debug.a
         MALLOC_DEBUG_LDFLAGS = -L$(PJDIR)/pjsip-apps/lib -Wl,-whole-archive -lasterisk_malloc_debug -Wl,-no-whole-archive
         ifeq ($(findstring DONT_OPTIMIZE,$(MENUSELECT_CFLAGS)),)
diff --git a/utils/Makefile b/utils/Makefile
index 6bd33da..167891e 100644
--- a/utils/Makefile
+++ b/utils/Makefile
@@ -172,6 +172,7 @@
 	$(CMD_PREFIX) cp "$<" "$@"
 
 
+extconf.o: _ASTCFLAGS+=$(call get_menuselect_cflags,DETECT_DEADLOCKS)
 extconf.o: extconf.c
 
 conf2ael: LIBS+=$(AST_CLANG_BLOCKS_LIBS)

-- 
To view, visit https://gerrit.asterisk.org/9806
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2085237a964f6e1e6fff55ed046e2afff83c027
Gerrit-Change-Number: 9806
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180814/e0b45aba/attachment-0001.html>


More information about the asterisk-code-review mailing list