From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 8AA5ADE5 for ; Mon, 30 Jan 2017 10:46:55 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 30 Jan 2017 01:46:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,311,1477983600"; d="scan'208";a="928111936" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by orsmga003.jf.intel.com with ESMTP; 30 Jan 2017 01:46:53 -0800 To: Thomas Monjalon References: <20170122015034.19824-1-ferruh.yigit@intel.com> <1800797.YCEvINBUOG@xps13> Cc: dev@dpdk.org From: Ferruh Yigit Message-ID: Date: Mon, 30 Jan 2017 09:46:53 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <1800797.YCEvINBUOG@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] mk: parallelize make config X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Jan 2017 09:46:56 -0000 On 1/29/2017 3:29 PM, Thomas Monjalon wrote: > 2017-01-22 01:50, Ferruh Yigit: >> make config dependency resolving was always running serial, >> parallelize it for better performance. > > It could be interesting to explain why it was not parallelized, > and how you made it possible. I will try to explain in next version. > > The test script should be updated as below: > > --- a/devtools/test-build.sh > +++ b/devtools/test-build.sh > - make T=$2 O=$1 config > + make -j$J T=$2 O=$1 config I will update. > > >> --- a/mk/internal/rte.depdirs-post.mk >> +++ b/mk/internal/rte.depdirs-post.mk >> @@ -29,11 +29,12 @@ >> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >> # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> >> -.PHONY: depdirs >> -depdirs: >> - @for d in $(DEPDIRS-y); do \ >> - $(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $$d ; \ >> - done >> +.PHONY: depdirs $(DEPDIRS-y) >> +depdirs: $(DEPDIRS-y) >> + @echo "" > > Why this echo "" ? If there is no dependency (DEPDIRS-y) set and there is nothing to do here, make prints a line like "nothing to do", since we redirect the output to the files, that msg goes into .depdir files. To prevent that msg, added a dummy action here. > >> + >> +$(DEPDIRS-y): >> + @$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $@ > > >> --- a/mk/rte.sdkdepdirs.mk >> +++ b/mk/rte.sdkdepdirs.mk >> @@ -36,19 +36,22 @@ ifeq (,$(wildcard $(RTE_OUTPUT)/Makefile)) >> $(error "need a make config first") >> endif >> >> +DEPDIRS = $(addsuffix /.depdirs, $(addprefix $(BUILDDIR)/,$(ROOTDIRS-y))) > > These DEPDIRS are files, although DEPDIRS in other contexts are directories. > I think it should be renamed. DEPDIR_FILES? Make sense, I will update. > >> # use a "for" in a shell to process dependencies: we don't want this >> # task to be run in parallel. > > You forgot to remove this obsolete comment. Will update on next version. > >> .PHONY: depdirs >> depdirs: $(RTE_OUTPUT)/.depdirs >> -$(RTE_OUTPUT)/.depdirs: $(RTE_OUTPUT)/.config >> - @rm -f $(RTE_OUTPUT)/.depdirs ; \ >> - for d in $(ROOTDIRS-y); do \ >> - if [ -f $(RTE_SRCDIR)/$$d/Makefile ]; then \ >> - [ -d $(BUILDDIR)/$$d ] || mkdir -p $(BUILDDIR)/$$d ; \ >> - $(MAKE) S=$$d -f $(RTE_SRCDIR)/$$d/Makefile depdirs \ >> - >> $(RTE_OUTPUT)/.depdirs ; \ >> - fi ; \ >> - done >> +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS) >> + @rm -f $@ >> + @for f in $(DEPDIRS); do cat $$f >> $@; done >> + @sort -u -o $@ $@ >> + >> +$(DEPDIRS): $(RTE_OUTPUT)/.config >> + @f=$(lastword $(subst /, ,$(dir $@))); \ > > Could you use $(notdir $(@D)) ? Can be, I will try. > >> + [ -d $(BUILDDIR)/$$f ] || mkdir -p $(BUILDDIR)/$$f; \ >> + rm -f $@; \ > > Why this removal? To be sure we are not using old files, but this can be done below line with using ">" I guess. > >> + $(MAKE) S=$$f -f $(RTE_SRCDIR)/$$f/Makefile depdirs >> $@ > > This part is a bit complicated. > Could it be simplified by better naming $f? I will try. > > >> --- a/mk/rte.subdir.mk >> +++ b/mk/rte.subdir.mk >> @@ -76,7 +76,7 @@ clean: _postclean >> # include .depdirs and define rules to order priorities between build >> # of directories. >> # >> -include $(RTE_OUTPUT)/.depdirs >> +-include $(RTE_OUTPUT)/.depdirs >> >> define depdirs_rule >> $(1): $(sort $(patsubst $(S)/%,%,$(LOCAL_DEPDIRS-$(S)/$(1)))) >> @@ -84,16 +84,15 @@ endef >> >> $(foreach d,$(DIRS-y),$(eval $(call depdirs_rule,$(d)))) >> >> +DEPDIRS = $(wildcard $(addprefix $(S)/,$(DIRS-y))) >> >> # use a "for" in a shell to process dependencies: we don't want this >> # task to be run in parallel. > > You forgot to remove this obsolete comment. Right, will update. > >> -.PHONY: depdirs >> -depdirs: >> - @for d in $(DIRS-y); do \ >> - if [ -f $(SRCDIR)/$$d/Makefile ]; then \ >> - $(MAKE) S=$S/$$d -f $(SRCDIR)/$$d/Makefile depdirs ; \ >> - fi ; \ >> - done >> +.PHONY: depdirs $(DEPDIRS) >> +depdirs: $(DEPDIRS) >> + >> +$(DEPDIRS): >> + @$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile depdirs >