Skip to content

Conversation

DavidEGrayson
Copy link

@DavidEGrayson DavidEGrayson commented Dec 26, 2022

This pull request speeds up incremental building of an application using the Pico SDK. I think the incremental build time is really important for productivity: you want to make a change to the code and then see the results a few seconds later without getting distracted by other things during long build times.

Status quo

On Windows under MSYS2, using version 1.4.0 of the pico-sdk (2e6142b), when I do an incremental build of the hello_pio example in commit a7ad171 of pico-examples, it takes about 5.9 seconds and here is the output:

$ time make | ts '[%.S]'
[48.859133] [  0%] Performing build step for 'ELF2UF2Build'
[49.452387] [100%] Built target elf2uf2
[49.700331] [  0%] No install step for 'ELF2UF2Build'
[50.031675] [  0%] Completed 'ELF2UF2Build'
[50.511903] [  0%] Built target ELF2UF2Build
[50.808588] [  0%] Performing build step for 'PioasmBuild'
[51.372992] [100%] Built target pioasm
[51.672983] [  0%] No install step for 'PioasmBuild'
[51.947172] [  0%] Completed 'PioasmBuild'
[52.510109] [  0%] Built target PioasmBuild
[52.791404] [  0%] Built target hello_pio_hello_pio_h
[53.350379] [  0%] Built target bs2_default
[53.651092] [ 50%] Built target bs2_default_padded_checksummed_asm
[54.030474] [100%] Built target hello_pio

real    0m5.940s
user    0m0.105s
sys     0m0.304s

In this 5.9 second build, it looks like nearly 4 seconds are being spent to check if elf2uf2 and pioasm need to be rebuilt.

This is actually the shortest possible build time because I did not change any code. If I modify pio/hello_pio/hello.c, the build time goes up to about 7 s. (These build times are not reliable and seem to vary by a second or so if I wait and try them later.)

Solution

This pull request removes the lines that say BUILD_AWAYS 1 # force dependency checking in the calls to ExternalProject_Add in tools/FindPioasm.cmake and tools/FindELF2UF2.cmake. With this change, an incremental build of hello_pio with no code changes takes 2.7 seconds:

$ time make | ts '[%.S]'
[54.426106] [  0%] Built target PioasmBuild
[54.721123] [  0%] Built target hello_pio_hello_pio_h
[55.045993] [  0%] Built target bs2_default
[55.316484] [ 50%] Built target bs2_default_padded_checksummed_asm
[55.625261] [ 50%] Built target ELF2UF2Build
[56.083132] [100%] Built target hello_pio

real    0m2.720s
user    0m0.090s
sys     0m0.105s

If there has been a code change, that time goes up to about 3.6 seconds.

It's not clear exactly why BUILD_ALWAYS is set to 1. The comment on that line is brief, and the code has been like that since the initial release of the Pico SDK. Note that with the BUILD_ALWAYS line removed, pioasm and elf2uf2 will not be automatically recompiled when the source code of those projects changes. To benefit from changes in those projects, users will have to remove their build directory and start again, or they can just go into build/pioasm or build/elf2uf2 and run make. I would hope that these utilities are stable enough that this will not cause major problems for most of the users, and the users would appreciate the faster build times. If you ever want to force everyone to rebuild those tools after a particularly important change, I would think you could do something like change the CMake target name (that should force everyone to rebuild those tools once, instead of making them rebuild them every time they compile their code). You could even bake the SDK version number into the CMake target name to make this process automatic (I haven't tried that).

Future work

By the way, there are some signs that the code in FindPioasm.cmake and FindELF2UF2.cmake are not really operating the way one would expect. Each file has three different if statements to check whether we have found/configured the external utility, and those checks could be reduced to one. (I succeeded in doing that on an unpublished branch.) Also, due to the way CMake variable and target scopes work, I found that the code in those files runs many times in a project like pico-examples that builds multiple RP2040 applications, even though you would think it would just run once. I might make another pull request later to further improve these files. Would there be any interest in allowing the use of external pioasm and elf2uf2 utilities found on the PATH, which is probably the best way to speed up incremental build times?

by not always building the external projects pioasm and elf2uf2.
@lurch
Copy link
Contributor

lurch commented Dec 27, 2022

Would there be any interest in allowing the use of external pioasm and elf2uf2 utilities found on the PATH, which is probably the best way to speed up incremental build times?

See #396 ?

@kilograham
Copy link
Contributor

i think i'd rather this behavior was guarded by a CMake var (i.e. you can turn off BUILD_ALWAYS 1 if you set the var)

@DavidEGrayson
Copy link
Author

DavidEGrayson commented Jan 23, 2023

OK. I'm not excited about working on that if it's an optional feature that people have to remember to turn on. I'd be happy to make a different pull request that lets the Pico SDK detect pioasm and elf2uf2 on the PATH and use them, and falls back to external projects with BUILD_ALWAYS 1 if those are not present. That would be a different way to speed up the builds and it would only require some work by the user once to install those utilities, instead of requiring them to set a CMake variable for every project they work on.

@DavidEGrayson
Copy link
Author

Building in MSYS2 is actually way faster if I use the Ninja generator, which is the default now (0.31 seconds for an incremental build of a project only using ELF2UF2). I'll just close this.

@lurch
Copy link
Contributor

lurch commented Feb 12, 2023

Does #1221 partially address this?

@DavidEGrayson
Copy link
Author

DavidEGrayson commented Feb 12, 2023

Yeah. Actually, even before #1221 is merged, it seems like it would be possible for you to install CMake packages named Pioasm and ELF2UF2, and CMake would probably find them and use them before it finds the ones defined in the SDK. I see that #1221 specifies a version number when finding the package though, so it probably gives us better control over which version of those utilities will get used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants