nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

Remove duplicate calls and optimize compilation speed

Open Gary-Hobson opened this issue 3 years ago • 3 comments

Summary

This commit reduces incremental compile (second compile) time from 3.2s to 0.8s for sim:ostest

config.h has too many duplicate script commands For folders without any changes, it still needs to execute the same script 5 times and the output is the same each time

Impact

Testing

No configuration required. The first compile time will be a little faster, the second will be much faster

The test command I used:

./tools/configure.sh sim:ostest 
time make   # first compile
time make   # second compile

Gary-Hobson avatar Sep 12 '22 07:09 Gary-Hobson

This weekend I tested an old NuttX version (from 2018) and it is impressive how the build system is faster now! So this kind of improvement is welcome!

acassis avatar Sep 12 '22 15:09 acassis

It works for me and I do see a speedup -- about 10 seconds saved on a full build (1 minute 15 seconds before, reduced down to 1 minute 5 seconds), and 5 seconds saved on a partial build (10 seconds before, reduced down to 5 seconds after). It is amazing how much time is wasted spawning a subshell!!

hartmannathan avatar Sep 12 '22 16:09 hartmannathan

LGTM with the following caveat: I only tested ARM builds with GCC, since that is the only platform for which I have a toolchain currently. Hopefully this doesn't break anything on any other platform or toolchain.

If someone has the ability to build the z80 target, or perhaps the sim target on Windows, that will give us some insight.

I think this should be safe. This change doesn't modify any data, it just remembers the result of the first output. Of course it would be better if someone could verify it

Gary-Hobson avatar Sep 13 '22 07:09 Gary-Hobson

This looks very promising. @Gary-Hobson any progress on this?

fjpanag avatar Oct 14 '22 11:10 fjpanag

This line of code looks ugly, but I didn't think of any proper solution It is the prefix used to get define

Do you guys have any better suggestions

export DEFINE_PREFIX:= $(subst __DEF__,,${shell $(DEFINE) "$(CC)" "__DEF__"})

Gary-Hobson avatar Oct 24 '22 15:10 Gary-Hobson

This looks very promising. @Gary-Hobson any progress on this?

Yes,I am perfecting it

Gary-Hobson avatar Oct 24 '22 15:10 Gary-Hobson

There is an issue with commit 85f727f23250df8fd0a29e7a67f60a316b1b1e8c.

I am running:

make  -C nuttx/tools -f Makefile.host clean

and I get the following errors:

make[1]: Entering directory '/path/to/workspace/nuttx/tools'
/bin/sh: line 1:/path/to/workspace/nuttx/tools/../tools/incdir: No such file or directory
/bin/sh: line 1: /path/to/workspace/nuttx/tools/../tools/incdir: No such file or directory

fjpanag avatar Nov 03 '22 15:11 fjpanag

make -C nuttx/tools -f Makefile.host clean

Can you tell with which configuration and steps this problem can be reproduced? I have not used this command before,I just tried it and it's fine

Gary-Hobson avatar Nov 03 '22 16:11 Gary-Hobson

I just tried this:

$ mkdir test
$ cd test
$ git clone https://github.com/apache/incubator-nuttx.git nuttx
$ git clone https://github.com/apache/incubator-nuttx-apps.git apps
$ make -C nuttx/tools -f Makefile.host clean

and I get the output:

make: Entering directory '/home/user/Downloads/test/nuttx/tools'
/bin/sh: line 1: /home/user/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
/bin/sh: line 1: /home/user/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
make[1]: Entering directory '/home/user/Downloads/test/nuttx/tools/pic32'
make[1]: Leaving directory '/home/user/Downloads/test/nuttx/tools/pic32'
make[1]: Entering directory '/home/user/Downloads/test/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/user/Downloads/test/nuttx/tools/cxd56'
make: Leaving directory '/home/user/Downloads/test/nuttx/tools'

You don't neeed any specific configuration to reproduce this.

fjpanag avatar Nov 03 '22 16:11 fjpanag

I don't think it's related to this commit,I run it without any problems

Your error appears to be an error deleting the file, but the incdir file didn't exist in the first place

You can execute make -C nuttx/tools -f Makefile.host clean V=1 to see the command executed in detail, or you can verify that deleting a non-existing file will give an error rm -f nuttx/tools/incdir

This is the result of my running:

rm -f  jlink-nuttx.so
rm -f  jlink-nuttx.dll
rm -rf *.dSYM
make -C pic32 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/pic32'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/pic32'
make -C cxd56 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/cxd56'
rm -f  incdir
rm -f  incdir.exe
rm -f *.o *.a *~ .*.swp   
make: Leaving directory '/home/user/NXOS/nuttx/tools'

Gary-Hobson avatar Nov 04 '22 03:11 Gary-Hobson

I don't think it's related to this commit,I run it without any problems

I ended up to this commit after bisecting the code.

You can execute make -C nuttx/tools -f Makefile.host clean V=1 to see the command executed in detail,

This is my output with V=1. Is it different than yours?

make: Entering directory '/home/fotis/Downloads/test/nuttx/tools'
/bin/sh: line 1: /home/fotis/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
/bin/sh: line 1: /home/fotis/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
rm -f  b16
rm -f  b16.exe
rm -f  bdf-converter
rm -f  bdf-converter.exe
rm -f  cmpconfig
rm -f  cmpconfig.exe
rm -f  cnvwindeps
rm -f  cnvwindeps.exe
rm -f  convert-comments
rm -f  convert-comments.exe
rm -f  configure
rm -f  configure.exe
rm -f  detab
rm -f  detab.exe
rm -f  gencromfs
rm -f  gencromfs.exe
rm -f  initialconfig
rm -f  initialconfig.exe
rm -f  lowhex
rm -f  lowhex.exe
rm -f  Make.dep
rm -f  mkconfig
rm -f  mkconfig.exe
rm -f  mkdeps
rm -f  mkdeps.exe
rm -f  mksymtab
rm -f  mksymtab.exe
rm -f  mksyscall
rm -f  mksyscall.exe
rm -f  mkversion
rm -f  mkversion.exe
rm -f  nxstyle
rm -f  nxstyle.exe
rm -f  rmcr
rm -f  rmcr.exe
rm -f  jlink-nuttx.so
rm -f  jlink-nuttx.dll
rm -rf *.dSYM
make -C pic32 -f Makefile.host clean
make[1]: Entering directory '/home/fotis/Downloads/test/nuttx/tools/pic32'
make[1]: Leaving directory '/home/fotis/Downloads/test/nuttx/tools/pic32'
make -C cxd56 -f Makefile.host clean
make[1]: Entering directory '/home/fotis/Downloads/test/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/fotis/Downloads/test/nuttx/tools/cxd56'
rm -f  incdir
rm -f  incdir.exe
rm -f *.o *.a *~ .*.swp   
make: Leaving directory '/home/fotis/Downloads/test/nuttx/tools'

or you can verify that deleting a non-existing file will give an error rm -f nuttx/tools/incdir

No, this does not produce an error, as expected.

fjpanag avatar Nov 04 '22 11:11 fjpanag

Are you sure that you are running make -C nuttx/tools -f Makefile.host clean V=1 on a recent NuttX revision?
This output does not correspond to the actual code within the makefile.

I don't think it's related to this commit,I run it without any problems

Your error appears to be an error deleting the file, but the incdir file didn't exist in the first place

You can execute make -C nuttx/tools -f Makefile.host clean V=1 to see the command executed in detail, or you can verify that deleting a non-existing file will give an error rm -f nuttx/tools/incdir

This is the result of my running:

rm -f  jlink-nuttx.so
rm -f  jlink-nuttx.dll
rm -rf *.dSYM
make -C pic32 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/pic32'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/pic32'
make -C cxd56 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/cxd56'
rm -f  incdir
rm -f  incdir.exe
rm -f *.o *.a *~ .*.swp   
make: Leaving directory '/home/user/NXOS/nuttx/tools'

fjpanag avatar Nov 04 '22 11:11 fjpanag

I don't think it's related to this commit,I run it without any problems

Your error appears to be an error deleting the file, but the incdir file didn't exist in the first place

You can execute make -C nuttx/tools -f Makefile.host clean V=1 to see the command executed in detail, or you can verify that deleting a non-existing file will give an error rm -f nuttx/tools/incdir

This is the result of my running:

rm -f  jlink-nuttx.so
rm -f  jlink-nuttx.dll
rm -rf *.dSYM
make -C pic32 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/pic32'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/pic32'
make -C cxd56 -f Makefile.host clean
make[1]: Entering directory '/home/user/NXOS/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/user/NXOS/nuttx/tools/cxd56'
rm -f  incdir
rm -f  incdir.exe
rm -f *.o *.a *~ .*.swp   
make: Leaving directory '/home/user/NXOS/nuttx/tools'

The problem is caused by the add of these two line: https://github.com/apache/incubator-nuttx/master/tools/Config.mk:L581~L582 export INCDIR_PREFIX := $(subst "X",,${shell $(INCDIR) "$(CC)" "X"}) export INCSYSDIR_PREFIX := $(subst "X",,${shell $(INCDIR) -s "$(CC)" "X"})

zouboan avatar Nov 06 '22 09:11 zouboan

The problem is caused by the add of these two line: https://github.com/apache/incubator-nuttx/master/tools/Config.mk:L581~L582 export INCDIR_PREFIX := $(subst "X",,${shell $(INCDIR) "$(CC)" "X"}) export INCSYSDIR_PREFIX := $(subst "X",,${shell $(INCDIR) -s "$(CC)" "X"})

Thanks, I just verified that it is indeed the reason

Analysis of specific reasons: When using clean code, the incdir file does not exist, but config.mk is referenced when executing the command, and incdir is used here, so the above error occurs.

Gary-Hobson avatar Nov 06 '22 09:11 Gary-Hobson

Has anyone verified if the Z80 compilers (ez8cc, zneocc, ez80cc) still work after this PR?

Asking because these compilers have a very different syntax that is not only a INCDIR_PREFIX or INCSYSDIR_PREFIX, but also includes other characters.

For example, while GCC uses:

-I<dir1> -I<dir2> -I<dir3>

the Z80 compilers use:

-usrinc:'dir1;dir2;dir3'

or

-stdinc:'dir1;dir2;dir3'

depending if user or system header.

With this PR, I think the command line will be:

-stdinc:dir

instead of:

-stdinc:'dir'

I don't know if that makes a difference.

hartmannathan avatar Nov 06 '22 14:11 hartmannathan

I just tried this:

$ mkdir test
$ cd test
$ git clone https://github.com/apache/incubator-nuttx.git nuttx
$ git clone https://github.com/apache/incubator-nuttx-apps.git apps
$ make -C nuttx/tools -f Makefile.host clean

and I get the output:

make: Entering directory '/home/user/Downloads/test/nuttx/tools'
/bin/sh: line 1: /home/user/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
/bin/sh: line 1: /home/user/Downloads/test/nuttx/tools/../tools/incdir: No such file or directory
make[1]: Entering directory '/home/user/Downloads/test/nuttx/tools/pic32'
make[1]: Leaving directory '/home/user/Downloads/test/nuttx/tools/pic32'
make[1]: Entering directory '/home/user/Downloads/test/nuttx/tools/cxd56'
make[1]: Leaving directory '/home/user/Downloads/test/nuttx/tools/cxd56'
make: Leaving directory '/home/user/Downloads/test/nuttx/tools'

You don't neeed any specific configuration to reproduce this.

The same problem occurs when executing checkpatch.sh in some cases Sorry, I caused this problem, I am fixing it

Gary-Hobson avatar Nov 07 '22 08:11 Gary-Hobson

It took me a while to find ez80cc in the old repository, confirmed that it can be used like this,Works fine with or without quotes

However, this only works for stdinc , I tried specifying any path with usrinc to no avail

https://github.com/CE-Programming/toolchain/releases/tag/v6.0

The latest (after v9.0) ez80cc has been based on llvm, and renamed to ez80-clang Should we also drop this ancient usage and use the latest ez80 toolchain

PS D:\CEdev\bin> cat main.c
#include <inc.h>
PS D:\CEdev\bin> cat ..\inc.h
#error "Invalid"
PS D:\CEdev\bin> .\ez80cc.exe -stdinc:".." main.c
main.c
..\INC.H        (1,7) : ERROR (38) ""Invalid" "
MAIN.C  (3,3) : WARNING (44) Empty file encountered
PS D:\CEdev\bin> .\ez80cc.exe -stdinc:.. main.c
main.c
..\INC.H        (1,7) : ERROR (38) ""Invalid" "
MAIN.C  (3,3) : WARNING (44) Empty file encountered
PS D:\CEdev\bin> .\ez80cc.exe -usrinc:".." main.c
main.c
MAIN.C  (1,9) : ERROR (217) Cannot open include file "inc.h"

Has anyone verified if the Z80 compilers (ez8cc, zneocc, ez80cc) still work after this PR?

Asking because these compilers have a very different syntax that is not only a INCDIR_PREFIX or INCSYSDIR_PREFIX, but also includes other characters.

For example, while GCC uses:

-I<dir1> -I<dir2> -I<dir3>

the Z80 compilers use:

-usrinc:'dir1;dir2;dir3'

or

-stdinc:'dir1;dir2;dir3'

depending if user or system header.

With this PR, I think the command line will be:

-stdinc:dir

instead of:

-stdinc:'dir'

I don't know if that makes a difference.

Gary-Hobson avatar Nov 08 '22 04:11 Gary-Hobson