[dpdk-dev] mk: parallelize make config
Checks
Commit Message
make config dependency resolving was always running serial,
parallelize it for better performance.
$ time make T=x86_64-native-linuxapp-gcc config
real 0m12.633s
$ time make -j8 T=x86_64-native-linuxapp-gcc config
real 0m1.826s
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
mk/internal/rte.depdirs-post.mk | 11 ++++++-----
mk/rte.sdkdepdirs.mk | 21 ++++++++++++---------
mk/rte.subdir.mk | 15 +++++++--------
3 files changed, 25 insertions(+), 22 deletions(-)
Comments
Hi Ferruh,
On Sun, 22 Jan 2017 01:50:34 +0000, Ferruh Yigit
<ferruh.yigit@intel.com> wrote:
> make config dependency resolving was always running serial,
> parallelize it for better performance.
>
> $ time make T=x86_64-native-linuxapp-gcc config
> real 0m12.633s
>
> $ time make -j8 T=x86_64-native-linuxapp-gcc config
> real 0m1.826s
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
I have a patch that fix the same issue (configuration takes to long),
but done differently. It is more intrusive, since it rework the way
DEPDIRS are used, but it does not require to use -j.
I'm sending it as a reply to this thread.
Regards,
Olivier
> On Jan 23, 2017, at 10:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Hi Ferruh,
>
> On Sun, 22 Jan 2017 01:50:34 +0000, Ferruh Yigit
> <ferruh.yigit@intel.com> wrote:
>> make config dependency resolving was always running serial,
>> parallelize it for better performance.
>>
>> $ time make T=x86_64-native-linuxapp-gcc config
>> real 0m12.633s
>>
>> $ time make -j8 T=x86_64-native-linuxapp-gcc config
>> real 0m1.826s
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
> I have a patch that fix the same issue (configuration takes to long),
> but done differently. It is more intrusive, since it rework the way
> DEPDIRS are used, but it does not require to use -j.
I tested the patch and the performance is very quick and seems to work very nicely. I have not looked at the other patch yet are they both compatible and could be combined or not?
>
> I'm sending it as a reply to this thread.
>
> Regards,
> Olivier
Regards,
Keith
2017-01-22 2:50 GMT+01:00 Ferruh Yigit <ferruh.yigit@intel.com>:
> make config dependency resolving was always running serial,
> parallelize it for better performance.
>
> $ time make T=x86_64-native-linuxapp-gcc config
> real 0m12.633s
>
> $ time make -j8 T=x86_64-native-linuxapp-gcc config
> real 0m1.826s
[...]
> +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS)
> + @rm -f $@
> + @for f in $(DEPDIRS); do cat $$f >> $@; done
> + @sort -u -o $@ $@
This could be just one 'sort -u -o $@ $(DEPDIRS)'
Best Regards,
Michał Mirosław
Hi Keith,
On Mon, 23 Jan 2017 17:50:27 +0000, "Wiles, Keith"
<keith.wiles@intel.com> wrote:
> > On Jan 23, 2017, at 10:18 AM, Olivier Matz <olivier.matz@6wind.com>
> > wrote:
> >
> > Hi Ferruh,
> >
> > On Sun, 22 Jan 2017 01:50:34 +0000, Ferruh Yigit
> > <ferruh.yigit@intel.com> wrote:
> >> make config dependency resolving was always running serial,
> >> parallelize it for better performance.
> >>
> >> $ time make T=x86_64-native-linuxapp-gcc config
> >> real 0m12.633s
> >>
> >> $ time make -j8 T=x86_64-native-linuxapp-gcc config
> >> real 0m1.826s
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >
> > I have a patch that fix the same issue (configuration takes to
> > long), but done differently. It is more intrusive, since it rework
> > the way DEPDIRS are used, but it does not require to use -j.
>
> I tested the patch and the performance is very quick and seems to
> work very nicely. I have not looked at the other patch yet are they
> both compatible and could be combined or not?
No, I don't think they could be combined.
Ferruh's approach looks less risky, since it's a parallelization
of the config. My approach changes the way dependencies are processed,
and also how they are declared. I think it's faster, but more intrusive.
Regards,
Olivier
On Tue, Jan 24, 2017 at 09:42:35AM +0100, Olivier MATZ wrote:
> Hi Keith,
>
> On Mon, 23 Jan 2017 17:50:27 +0000, "Wiles, Keith"
> <keith.wiles@intel.com> wrote:
> > > On Jan 23, 2017, at 10:18 AM, Olivier Matz <olivier.matz@6wind.com>
> > > wrote:
> > >
> > > Hi Ferruh,
> > >
> > > On Sun, 22 Jan 2017 01:50:34 +0000, Ferruh Yigit
> > > <ferruh.yigit@intel.com> wrote:
> > >> make config dependency resolving was always running serial,
> > >> parallelize it for better performance.
> > >>
> > >> $ time make T=x86_64-native-linuxapp-gcc config
> > >> real 0m12.633s
> > >>
> > >> $ time make -j8 T=x86_64-native-linuxapp-gcc config
> > >> real 0m1.826s
> > >>
> > >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > >
> > > I have a patch that fix the same issue (configuration takes to
> > > long), but done differently. It is more intrusive, since it rework
> > > the way DEPDIRS are used, but it does not require to use -j.
> >
> > I tested the patch and the performance is very quick and seems to
> > work very nicely. I have not looked at the other patch yet are they
> > both compatible and could be combined or not?
>
> No, I don't think they could be combined.
>
> Ferruh's approach looks less risky, since it's a parallelization
> of the config. My approach changes the way dependencies are processed,
> and also how they are declared. I think it's faster, but more intrusive.
>
I think we may hit a case of diminishing returns. On my system, the
config time goes from 12 seconds down to less than 1 second (large
amounts of parallelism). Even with limiting parallelism to just 4
make processes, the time for config drops to 3 seconds.
I don't see huge value in trying to shave off any more time than
that, given the amount of extra complexity involved.
/Bruce
On Sun, Jan 22, 2017 at 01:50:34AM +0000, Ferruh Yigit wrote:
> make config dependency resolving was always running serial,
> parallelize it for better performance.
>
> $ time make T=x86_64-native-linuxapp-gcc config
> real 0m12.633s
>
> $ time make -j8 T=x86_64-native-linuxapp-gcc config
> real 0m1.826s
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Works great for me! Config time has always been a pain.
Tested-by: Bruce Richardson <bruce.richardson@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
On 1/23/2017 7:03 PM, Michał Mirosław wrote:
> 2017-01-22 2:50 GMT+01:00 Ferruh Yigit <ferruh.yigit@intel.com>:
>> make config dependency resolving was always running serial,
>> parallelize it for better performance.
>>
>> $ time make T=x86_64-native-linuxapp-gcc config
>> real 0m12.633s
>>
>> $ time make -j8 T=x86_64-native-linuxapp-gcc config
>> real 0m1.826s
> [...]
>> +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS)
>> + @rm -f $@
>> + @for f in $(DEPDIRS); do cat $$f >> $@; done
>> + @sort -u -o $@ $@
>
> This could be just one 'sort -u -o $@ $(DEPDIRS)'
Yes, I will update in next version, thanks.
>
> Best Regards,
> Michał Mirosław
>
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
>
@@ -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 ""
+
+$(DEPDIRS-y):
+ @$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $@
.PHONY: depgraph
depgraph:
@@ -36,19 +36,22 @@ ifeq (,$(wildcard $(RTE_OUTPUT)/Makefile))
$(error "need a make config first")
endif
+DEPDIRS = $(addsuffix /.depdirs, $(addprefix $(BUILDDIR)/,$(ROOTDIRS-y)))
+
# use a "for" in a shell to process dependencies: we don't want this
# task to be run in parallel.
.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 $@))); \
+ [ -d $(BUILDDIR)/$$f ] || mkdir -p $(BUILDDIR)/$$f; \
+ rm -f $@; \
+ $(MAKE) S=$$f -f $(RTE_SRCDIR)/$$f/Makefile depdirs >> $@
.PHONY: depgraph
depgraph:
@@ -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.
-.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
.PHONY: depgraph
depgraph: