Enabled LTO (and whitespace fixes)

[Avamander]

Basically, the compiler we're using has a bug that makes LTO not work properly with weak symbols.

It's more reasonable to simply upgrade.

Before:

Memory region         Used Size  Region Size  %age Used
           FLASH:      394628 B       480 KB     80.29%
             RAM:       54768 B        64 KB     83.57%

After:

Memory region         Used Size  Region Size  %age Used
           FLASH:      371812 B       480 KB     75.65%
             RAM:       54760 B        64 KB     83.56%

Note: Recovery build has not been tested, pinetime-app, pinetime-mcuboot-app seem to work.

[evergreen22]

Does this PR close or obsolete #495 ?

[Avamander]

I don't think so.

[Avamander]

I see a lot of potential to further improve the set of optimization flags and the CMakefile (as done in https://github.com/InfiniTimeOrg/InfiniTime/pull/495).

However, I don't want to do it with this PR, it is already large as it is and pr-rot will soon get it unless merged.

[Avamander]

@geekbozu If you could take a look if everything seems fine, that'd be nice.

[Avamander]

@FintasticMan on Discord reported they're getting freezes. Investigating.

[JF002]

It appears that FreeRTOS heap and the stack of the main task (systemtask) are probably overflown. I increased the size of the heap (18*1024), the size of the stack of SystemTask and disabled the HeartRate task (to free a bit of RAM) in this DFU : pinetime-mcuboot-app-dfu-1.7.98.zip This DFU also has RTT logging enabled, btw.

Patch : Test_LTO.txt

I'm running it right now on my sealed device, no issue to report right now.

---

When trying this branch, I also noticed a few issue, mainly with the Docker build : The cmake version embedded in the container is too old and cannot build the project and this URL (https://github.com/InfiniTimeOrg/InfiniTime/blob/cb5fefc590fa665213a1e9aadf2b634884abd15d/docker/build.sh#L42) does not exist.

[ManuelBoe14]

I'm running it right now on my sealed device, no issue to report right now.

I have also installed version 1.7.98 on my Pinetime. So far everything seems to be working.

[trman]

i installed 1.7.98 and the hr data no longer work (i need it for fitotrack) no improvement in speed or battery whatsoever , on other part of infinitme (except the now no functional heat rate) the dfu is near the limit of the pinetime too

do you plan to put back , the Heart rate task, before merging this pr for 1.8.0 release @JF002 ?

if anything , and if it's truly needed to remove a task , it might be better to remove something else than a core functionality of the pinetime ( the core functionality a the functionality that have a physical counterpart : steps (step counter hw ) / hr (green led HW)).

i see that the 1.7.98 is far bigger than usual ....

do that mean that enabling lto will forbid to add new functionality (like the near completed calculator of @Raupinger )?

for al i see , and if this the case this pr should not been merged until it doesn't need to remove functionality and doesn't take more space for no visible /perceptible improvement .

if the pr were to be merge as is by removing functionality and making impossible to add new one with removing existing app it would be more a big regression than an improvement!!

by the way , i need to go in recovery in order to flash dfu because , in secured bluetooth , i can't no longer do a dfu update .... is it related of this pr ? if not it shoiuld be resolved as well before 1.8.0 because , i don't think user will like no be able to update dfu ....

[trman]

i installed 1.78 and the hr data no longer work

@trman version 1.7.98 is for testing purposes only. JF002 has noted in his post above that he has deactivated the hr.

i do understand this , but he removed it becaause otherwise the dfu would not work ..... that's why iw wish that he don't use this 'trick' to validate this pr .... because i would not be able to use fitotrack

[Avamander]

@trman As written above, HR task was disabled.

If you need HR, don't flash random test builds meant to demonstrate something wholly different. It is not meant for regular usage and wasn't labeled as such. Any qualms about a random test build are really not relevant here and such test builds shouldn't be taken as something that is going to be instantly released.

Changes made are not all also immediately perceptible to the user, or really should be, there are other things that matter here as well.

Any issues you have with DFU are not relevant to this PR. You should really be expecting rough corners on something unreleased, it is unreleased for a reason. If you actually discover a bug that hasn't been reported, neither under that feature's PR or in an issue, then open a new one for that single bug.

[JF002]

@trman I'll give a bit of context about that 1.7.98 version. This is really a test build I built to check a single assumption : does increasing the heap of FreeRTOS and the stack size of SystemTask solve our memory corruption issue. The only purpose of this test build was to check the stability of InfiniTime with LTO enabled and the increased RAM allocation. Nothing else.

You've probably noticed : I didn't create a Pull-Request with those changes, because I don't want them to be merged.

But, as I had to free a bit of RAM to increase the heap/stack, I had to disable the HR task so I can use the RAM memory allocated to this task for other purposes. Again, I did this only for the purpose of this test and I won't merge that in develop/master

do that mean that enabling lto will forbid to add new functionality (like the near completed calculator of @Raupinger )?

The FLASH and RAM memories are already almost full, and that's why I cannot merge as many PR as I would like : I want to keep the memory usage under control, and some of these PRs have a lot of impact on the memory usage. The goal of LTO is to reduce the binary size (FLASH usage) so we can include some more functionalities. But, for some reason, using LTO seems to increase the RAM usage, and we still have to understand why and see what we can do about this.

by the way , i need to go in recovery in order to flash dfu because , in secured bluetooth , i can't no longer do a dfu update .... is it related of this pr ? if not it shoiuld be resolved as well before 1.8.0 because , i don't think user will like no be able to update dfu ....

This is not related to this PR, please see this

[JF002]

Here's a TODO list so we know what must still be done in this PR:

• [ ] Measure the gain in FLASH space
• [ ] Analyze RAM memory usage, adapt FreeRTOS heap and tasks stack size so they don't overflow. Try to provide an explanation why the RAM usage increased when enabling LTO
• [ ] Fix DEBUG build
• [ ] Fix Docker build
• [x] Check Github Actions build
• [ ] Check the build warnings returned by the compiler and fix them if relevant

[JF002]

@Avamander The test build ran for more than 6 days on my sealed unit (until I need to reboot to test other features). So I think it's quite stable once the stacks/heaps are correctly set :+1:

[trman]

any news on the remaining task?