Skip to content

Makefile.include: only keep macros in riotbuild.h#12348

Merged
kaspar030 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/remove_comments_riotbuild.h
Oct 1, 2019
Merged

Makefile.include: only keep macros in riotbuild.h#12348
kaspar030 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/remove_comments_riotbuild.h

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Oct 1, 2019 •
edited
Loading

Contribution description

Rely on creating an intermediate riotbuild.h.in file that is updated on
CFLAGS changes, but then generate 'riotbuild.h' without the comments.
The updated timestamp will still trigger a rebuild but not cause ccache
miss due to the content of the CFLAGS that contains absolute path.

This removes the caching issue due to the absolute path that was
added to CFLAGS and so the comment in that file.

Testing procedure

Low level testing procedure

The file riotbuild.h does not contain any absolute path anymore in a comment.

File is not updated on rebuilding the same thing but is on CFLAGS change:

RIOT_CI_BUILD=1 make -C examples/hello-world/ >/dev/null
stat --format='%y' examples/hello-world/bin/native/riotbuild/riotbuild.h
stat --format='%y' examples/hello-world/bin/native/application_hello-world/main.o
sleep 1s
RIOT_CI_BUILD=1 make -C examples/hello-world/ >/dev/null
echo It should be the same timestamps
stat --format='%y' examples/hello-world/bin/native/riotbuild/riotbuild.h
stat --format='%y' examples/hello-world/bin/native/application_hello-world/main.o
md5sum examples/hello-world/bin/native/riotbuild/riotbuild.h
# Then modify CFLAGS without modifying macros
sleep 1s
CFLAGS=-Werror RIOT_CI_BUILD=1 make -C examples/hello-world/ >/dev/null
echo Timestamp changed but md5sum stay the same
stat --format='%y' examples/hello-world/bin/native/riotbuild/riotbuild.h
stat --format='%y' examples/hello-world/bin/native/application_hello-world/main.o
md5sum examples/hello-world/bin/native/riotbuild/riotbuild.h

The output shows that on the first rebuild, both the header and the object were not changed, but on the rebuild with a different CFLAGS, it was rebuild despite still having the same content in riotbuild.h.

2019-10-01 11:11:21.715478333 +0200
2019-10-01 11:11:21.735478454 +0200
It should be the same timestamps
2019-10-01 11:11:21.715478333 +0200
2019-10-01 11:11:21.735478454 +0200
9b0ced6bc25fde0ca2c25922cad8069b  examples/hello-world/bin/native/riotbuild/riotbuild.h
Timestamp changed but md5sum stay the same
2019-10-01 11:11:25.411500357 +0200
2019-10-01 11:11:25.431500476 +0200
9b0ced6bc25fde0ca2c25922cad8069b  examples/hello-world/bin/native/riotbuild/riotbuild.h
The file content is the same as before without the `CFLAGS` comment: 
cat examples/hello-world/bin/native/riotbuild/riotbuild.h
/* Generated file do not edit */
#define DEVELHELP 1
#define DEBUG_ASSERT_VERBOSE 1
#define RIOT_APPLICATION "hello-world"
#define BOARD_NATIVE "native"
#define RIOT_BOARD BOARD_NATIVE
#define CPU_NATIVE "native"
#define RIOT_CPU CPU_NATIVE
#define MCU_NATIVE "native"
#define RIOT_MCU MCU_NATIVE
#define RIOT_VERSION "buildtest"
#define MODULE_AUTO_INIT 1
#define MODULE_BOARD 1
#define MODULE_CORE 1
#define MODULE_CORE_MSG 1
#define MODULE_CPU 1
#define MODULE_NATIVE_DRIVERS 1
#define MODULE_PERIPH 1
#define MODULE_PERIPH_COMMON 1
#define MODULE_PERIPH_GPIO 1
#define MODULE_PERIPH_PM 1
#define MODULE_PERIPH_UART 1
#define MODULE_SYS 1

High level testing procedure

Changing a macro like the RIOT_VERSION without cleaning still re-builds:

Hello-world with different RIOT_VERSION 
RIOT_CI_BUILD=1 make -C examples/hello-world/ flash term 
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "native" with MCU "native".

   text    data     bss     dec     hex filename
  22918     568   47652   71138   115e2 /home/harter/work/git/RIOT/examples/hello-world/bin/native/hello-world.elf
true 
/home/harter/work/git/RIOT/examples/hello-world/bin/native/hello-world.elf  
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: buildtest)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

And then just after

RIOT_VERSION=lalala RIOT_CI_BUILD=1 make -C examples/hello-world/ flash term 
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "native" with MCU "native".

   text    data     bss     dec     hex filename
  22914     568   47652   71134   115de /home/harter/work/git/RIOT/examples/hello-world/bin/native/hello-world.elf
true 
/home/harter/work/git/RIOT/examples/hello-world/bin/native/hello-world.elf  
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: lalala)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

Caching impact

@kaspar030 testing procedure for the caching

Issues/PRs references

#12262 (comment)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Oct 1, 2019

There's a typo in the commit message ;)

@cladmi
Makefile.include: only keep macros in riotbuild.h
3bfe30a 
Rely on creating an intermediate riotbuild.h.in file that is updated on
CFLAGS changes, but then generate 'riotbuild.h' without the comments.
The updated timestamp will still trigger a rebuild but not cause ccache
miss due to the content of the CFLAGS that contains absolute path.

This removes the caching issue due to the absolute path that was
added to `CFLAGS` and so the comment in that file.
@cladmi cladmi force-pushed the pr/make/remove_comments_riotbuild.h branch from 0af1161 to 3bfe30a Compare October 1, 2019 09:26
@cladmi cladmi changed the title Makefile.incule: only keep macros in riotbuild.h Makefile.include: only keep macros in riotbuild.h Oct 1, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Oct 1, 2019

Fixed, I only re-read the content and not the title ><

@cladmi cladmi mentioned this pull request Oct 1, 2019
Merged
@benpicco benpicco added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Oct 1, 2019
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Oct 1, 2019

@kaspar030 testing procedure for the caching

This seems to recover the caching situation.

Tihs PR:

first run...                                                                                                                       
[3m:43s] 602/602 jobs done (602 passed, 0 failed.) ETA: 0s                                                                         
ccache stats:                                                                                                                      
cache directory                     /data/riotbuild/.ccache                                                                        
primary config                      /data/riotbuild/.ccache/ccache.conf                                                            
secondary config      (readonly)    /etc/ccache.conf                                                                               
stats zero time                     Tue Oct  1 08:59:38 2019                                                                       
cache hit (direct)                     0                                                                                           
cache hit (preprocessed)           33243                                                                                           
cache miss                          8163                                                                                           
cache hit rate                     80.29 %                                                                                         
cleanups performed                    16                                                                                           
files in cache                     49798                                                                                           
cache size                         248.4 MB                                                                                        
max files                        3500000                                                                                           
max cache size                      14.0 GB                                                                                        
second run                                                                                                                         
[2m:06s] 602/602 jobs done (602 passed, 0 failed.) ETA: 0s                                                                         
ccache stats:                                                                                                                      
cache directory                     /data/riotbuild/.ccache                                                                        
primary config                      /data/riotbuild/.ccache/ccache.conf                                                            
secondary config      (readonly)    /etc/ccache.conf                                                                               
stats zero time                     Tue Oct  1 08:59:38 2019                                                                       
cache hit (direct)                 33839                                                                                           
cache hit (preprocessed)           40806                                                                                           
cache miss                          8167                                                                                           
cache hit rate                     90.14 %                                                                                         
cleanups performed                    16                                                                                           
files in cache                     51639                                                                                           
cache size                         256.2 MB                                                                                        
max files                        3500000                                                                                           
max cache size                      14.0 GB

This PR's mergebase:

first run...                                                                                                               [75/236]
[4m:07s] 602/602 jobs done (602 passed, 0 failed.) ETA: 0s                                                                         
ccache stats:                                                                                                                      
cache directory                     /data/riotbuild/.ccache                                                                        
primary config                      /data/riotbuild/.ccache/ccache.conf                                                            
secondary config      (readonly)    /etc/ccache.conf                                                                               
stats zero time                     Tue Oct  1 09:08:59 2019                                                                       
cache hit (direct)                     0                                                                                           
cache hit (preprocessed)           33254                                                                                           
cache miss                          8152                                                                                           
cache hit rate                     80.31 %                                                                                         
cleanups performed                    16                                                                                           
files in cache                     49804                                                                                           
cache size                         248.1 MB                                                                                        
max files                        3500000                                                                                           
max cache size                      14.0 GB                                                                                        
second run                                                                                                                         
[2m:55s] 602/602 jobs done (602 passed, 0 failed.) ETA: 0s                                                                         
ccache stats:                                                                                                                      
cache directory                     /data/riotbuild/.ccache                                                                        
primary config                      /data/riotbuild/.ccache/ccache.conf                                                            
secondary config      (readonly)    /etc/ccache.conf                                                                               
stats zero time                     Tue Oct  1 09:08:59 2019                                                                       
cache hit (direct)                  1716                                                                                           
cache hit (preprocessed)           72940                                                                                           
cache miss                          8156                                                                                           
cache hit rate                     90.15 %                                                                                         
cleanups performed                    16                                                                                           
files in cache                     52586                                                                                           
cache size                         311.8 MB                                                                                        
max files                        3500000                         
max cache size                      14.0 GB

Something else is off, two days ago there were 15k less files and ~50MB less data in cache after one build. But I think that's unrelated.

kaspar030
kaspar030 approved these changes Oct 1, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

Recovers Murdock caching to previous levels. sed'ing out the comment seems an OK solution.

@kaspar030 kaspar030 merged commit 1663c49 into RIOT-OS:master Oct 1, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Oct 1, 2019

Thanks for the review. I will do a test with removing completely the macros from that file.

@cladmi cladmi deleted the pr/make/remove_comments_riotbuild.h branch October 1, 2019 11:29
@fhessel fhessel mentioned this pull request Oct 13, 2019
Merged
@fhessel fhessel mentioned this pull request Oct 28, 2019
Merged
@kb2ma kb2ma added this to the Release 2019.10 milestone Oct 28, 2019
@cladmi cladmi mentioned this pull request Oct 30, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 kaspar030 approved these changes

Assignees

No one assigned

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.10

Development

Successfully merging this pull request may close these issues.

5 participants

@cladmi @aabadie @kaspar030 @benpicco @kb2ma