From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH] mk: parallelize make config
Date: Sun, 29 Jan 2017 16:29:35 +0100 [thread overview]
Message-ID: <1800797.YCEvINBUOG@xps13> (raw)
In-Reply-To: <20170122015034.19824-1-ferruh.yigit@intel.com>
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.
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
> --- 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 "" ?
> +
> +$(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?
> # 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.
> .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)) ?
> + [ -d $(BUILDDIR)/$$f ] || mkdir -p $(BUILDDIR)/$$f; \
> + rm -f $@; \
Why this removal?
> + $(MAKE) S=$$f -f $(RTE_SRCDIR)/$$f/Makefile depdirs >> $@
This part is a bit complicated.
Could it be simplified by better naming $f?
> --- 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.
> -.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
next prev parent reply other threads:[~2017-01-29 15:29 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-22 1:50 [PATCH] mk: parallelize make config Ferruh Yigit
2017-01-23 17:18 ` Olivier Matz
2017-01-23 17:19 ` [PATCH] mk: optimize directory dependencies Olivier Matz
2017-01-24 11:19 ` Robin Jarry
2017-01-24 11:26 ` Bruce Richardson
2017-01-24 12:31 ` Robin Jarry
2017-01-24 11:40 ` Jerin Jacob
2017-01-24 12:15 ` Bruce Richardson
2017-01-24 12:56 ` Jerin Jacob
2017-01-24 13:26 ` Richardson, Bruce
2017-01-24 14:50 ` Olivier MATZ
2017-01-24 14:55 ` Wiles, Keith
2017-03-01 11:25 ` Thomas Monjalon
2017-03-01 12:10 ` Bruce Richardson
2017-03-01 12:30 ` Olivier Matz
2017-01-24 13:05 ` Ferruh Yigit
2017-03-17 17:13 ` Olivier Matz
2017-03-17 17:47 ` Robin Jarry
2017-03-20 8:31 ` Olivier Matz
2017-03-24 13:21 ` [PATCH v2] " Olivier Matz
2017-03-27 21:33 ` Thomas Monjalon
2017-03-28 10:34 ` Ferruh Yigit
2017-03-30 8:51 ` Olivier Matz
2017-03-30 9:27 ` Ferruh Yigit
2017-03-30 12:11 ` Olivier Matz
2017-03-30 12:32 ` [PATCH] mk: fix dependencies to optional configs Olivier Matz
2017-03-30 12:37 ` Ferruh Yigit
2017-03-30 13:37 ` Thomas Monjalon
2017-01-23 17:50 ` [PATCH] mk: parallelize make config Wiles, Keith
2017-01-24 8:42 ` Olivier MATZ
2017-01-24 10:02 ` Bruce Richardson
2017-01-23 19:03 ` Michał Mirosław
2017-01-30 9:41 ` Ferruh Yigit
2017-01-24 10:52 ` Bruce Richardson
2017-01-29 15:29 ` Thomas Monjalon [this message]
2017-01-30 9:46 ` Ferruh Yigit
2017-01-30 10:21 ` [PATCH v2] " Ferruh Yigit
2017-01-30 18:13 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1800797.YCEvINBUOG@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.