You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/11/18 16:52:09 UTC

[GitHub] [incubator-nuttx] v01d opened a new pull request #2335: Parallelize depend file generation

v01d opened a new pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335


   ## Summary
   
   This is a WIP to paralellize generation of Make.dep files. It does so by generating temporary per-file dep file as a separate make target (which allows parallelization) which are collected into the final Make.dep.
   
   This is a first naive attempt for testing (and only applied to sim arch Makefile, every other arch would need the same treatment). I would like to know about anything that may not work in other platforms (I test on Linux) and a better way to handle all this repetition. Ideally, most of these rules could be centralized somewhere but there are many slight variations of the dependency call which I'm not sure what would be the best way to handle compactly.
   
   ## Impact
   
   Dependency generation speed improvment. 
   
   * Original version:
      * Single job:
         make depend  16.47s user 8.86s system 102% cpu 24.790 total
      * Multiple jobs (will only paralellize across directories):
         make depend -j8  16.11s user 9.02s system 118% cpu 21.163 total
   * Modified version:
     * Multiple jobs (I have four actual cores, eight via hyperthreading):
        make depend -j8  19.35s user 7.85s system 323% cpu 8.399 total
   
   ## Testing
   
   sim:nsh
   
   Ideally testing should verify we get the same Make.dep files.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#issuecomment-731249783


   I just applied the change to other archs and some missing directories. Does someone want to give this a try?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526316225



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       No we don't need, all Makefile can keep as before, in mkdeps:
   
   1. Launch gcc in parallel to generate the individual dep file
   2. Wait all gcc instance finish, and merge small dep files into the big one
   
   Basically, the change do the same thing in mkdeps.c as you modify in each Makefile.
   But since the input to and output from mkdeps is same as before, no change to Makefile is required.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] acassis commented on pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#issuecomment-731644509


   Hi @v01d since you marked this PR as ready for review, please remove the WIP from Summary test


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526332302



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       But that is what is doing right now AFAIK. And GCC does not parallelize internally.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526316225



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       No we don't need, all Makefile can keep as before, in mkdeps:
   
   1. Launch gcc in parallel to generate the individual dep file
   2. Wait all gcc instance finish, and merge small dep files into the big one
   
   Basically, the change do the same thing in mkdeps.c as your modification in each Makefile.
   But since the input to and output from mkdeps is same as before, no any change to Makefile is required.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526316225



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       No we don't need, all Makefile can keep as before, in mkdeps:
   
   1. launch gcc in parallel to generate the individual dep file
   2. Wait all gcc instance finish, and merge small dep files into the big one
   
   Basically, the change do the same thing in mkdeps.c as you modify in each Makefile.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526302883



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       if we still use Make.dep which contain all dependence info as before. The simplest approach is to enhance MKDEP to launch $(CC) in parallel:
   https://github.com/apache/incubator-nuttx/blob/master/tools/mkdeps.c#L972-L981
   then we don't need touch so many Makefile.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526320269



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       I considered that initially, but in that case we're getting into OS dependant handling of parallelization. Make already handles how to run the jobs in parallel and also deals with the -j flag (also via MAKEFLAGS). We would be redoing a lot of that.
   I prefer to do a Make based solution. 
   Maybe I can define per-file .dd targets, which would not require one pattern per case (something in the lines of how targets are defined for applications).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526305597



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       But we need to define the new targets as well and as you can see they are slightly different in different cases (according to flags, bin path) and at least the way I found to handle that is to use different dep file extensions for each case (so one target for each).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526302883



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       if we still use Make.dep which contain all dependence info as before. The simplest approach is to enhance MKDEP to launch $(CC) in parallel, then we don't touch so many Makefile.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526316225



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       No we don't need, all Makefile can keep as before, in mkdeps:
   
   1. launch gcc in parallel to generate the individual dep file
   2. Wait all gcc instance finish, and merge small dep files into the big one
   
   Basically, the change do the same thing in mkdeps.c as you modify in each Makefile.
   But since the input to and output from mkdeps is same as before, no change to Makefile is required.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526561425



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       Yes, you are right.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526302883



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       if we still use Make.dep which contain all dependence info as before. The simplest approach is to enhance MKDEP to launch $(CC) in parallel, then we don't need touch so many Makefile.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526979775



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       Ok, I think I managed to simplify it quite a bit and looks like a reasonable solution to me. Still needs the same application in all archs.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526331509



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       If you don't want to manage multiple threads manually, we can invoke gcc with all source files once and let gcc do parallel for us.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526921141



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       The rule I was mentioning would be something like:
   ```
   $(ASRCS:.S=.dd): %.dd: %.S
   	$(Q) $(MKDEP) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
   ```
   and same for CSRCS, CXXSRCS, HOSTSRCS. I can handle the kernel/non-kernel by having the depend target
   inside bin/kbin, passing KDEFINE or not in each case, so that is no issue.
   
   The problem with this, is that if I want to have these rules in Config.mk, I would need to create a template (like SDIR_template) and do `eval` on each Makefile so that these rules are defined after ASRCS and others are already defined. Otherwise, these rules would be evaluated while these variables are empty. 
   I think it is not actually worse than the various dep-related rules we have in each Makefile already, but it feels a bit of an ugly solution (specially since I hate having to remember to do this on each new Makefile and will fail silently if you forget).
   
   The only alternative I can think of is to have one dep file extension for each case (eg: .dds, .ddc, .ddx, .ddh) which can then be defined in Config.mk directly. These files are temporary so I don't think it is much of an issue but also feels a bit of a hack.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] v01d commented on pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#issuecomment-731648188


   > Hi @v01d since you marked this PR as ready for review, please remove the WIP from Summary test
   
   Ok, done


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2335: Parallelize depend file generation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2335:
URL: https://github.com/apache/incubator-nuttx/pull/2335#discussion_r526331509



##########
File path: boards/Makefile
##########
@@ -98,13 +98,28 @@ $(CXXOBJS): %$(OBJEXT): %.cxx
 
 $(BIN): $(OBJS)
 	$(call ARCHIVE, $@, $(OBJS))
+  
+%.dd: %.c
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+  
+%.ddxx: %.cxx
+	$(Q) $(MKDEP) $(DEPPATH) "$(CC)" -- $(CXXFLAGS) -- $< > $@
+
+makedepfile: $(patsubst %.c,%.dd,$(SRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
+
+makedepxxfile: $(patsubst %.cxx,%.ddxx,$(CXXSRCS))
+	@for f in $^; do cat $$f; done >> Make.dep
+	$(call DELFILE, $^)
 
 .depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
+	$(call DELFILE, Make.dep)

Review comment:
       If you don't want to manage multiple threads manually, we can invoke gcc with all source files once and let gcc do parallel for us.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org