Skip to content

Make SPIFFS and LittleFS stay out of link when not needed #6699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 7, 2019

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Nov 2, 2019

  • define two weak functions defaulting to no-op
  • redefine them to do something useful when either spiffs or littlefs are used/linked

This comes after a discussion with @earlephilhower
(who will be too busy for the next days to come to continue on this)

replaces and closes #6697
fixes #6691

Tested with all 4 uncommented ets_printf from this PR,
calling the two weak functions in setup(), and calling or not SPIFFS.begin() / LittleFS.begin().

d-a-v added 2 commits November 2, 2019 01:04
redefine them to do something useful when either spiffs or littlefs are used
@TD-er
Copy link
Contributor

TD-er commented Nov 2, 2019

I've looked at the code of this PR and I do think I get how it should work.
I'm not entirely sure about why it will not link the strong function implementations, but that's more lack of knowledge on my part I guess.

Do you already have any idea on how much smaller a SPIFFS-only (or LittleFS-only) build is with this fix?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 2, 2019

SPIFFS is 32KB and LittleFS 25KB as far as I remember.

The trick in this PR and WPS is that there are .cpp.o files always linked in which we put the weakref functions. Other .cpp.o are indeed compiled but may not be included because their symbols are not needed (no dependencies). Once a dependency is forced in a linker unit (ie. call to someInstance.begin(), all of its symbols come to the final binary, and weak functions are overridden at that time if their new implementation are brought with.

@TD-er
Copy link
Contributor

TD-er commented Nov 2, 2019

But why doesn't it try to link in the .o file if it does find the strong function implementation in there (and nothing else is needed)
Is it purely because the weak link already is resolved by something that is guaranteed to be already processed? How is that guaranteed?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 2, 2019

But why doesn't it try to link in the .o file if it does find the strong function implementation in there (and nothing else is needed)

I don't know that for sure.

Is it purely because the weak link already is resolved by something that is guaranteed to be already processed? How is that guaranteed?

What I can say is that close_all_fs() needs to be linked, and it brings the weak definitions with it.

@devyte
Copy link
Collaborator

devyte commented Nov 2, 2019

@TD-er it links either both, in which case the weak is discarded due to the presence of the strong, or just the weak, in which case the weak is used (and optimized out because it's an empty function).

@TD-er
Copy link
Contributor

TD-er commented Nov 2, 2019

@TD-er it links either both, in which case the weak is discarded due to the presence of the strong, or just the weak, in which case the weak is used (and optimized out because it's an empty function).

That's something I know, but what I don't know is how it is guaranteed the weak one is found first before the strong one, when the strong one is not needed.
The strong one will also be compiled, but the other functions in the .o file will not be needed when not called. So either linking the strong one or the weak one will resolve the link requirements, so I can imagine it depends on what order the function will be found by the linker.
Or am I missing something here?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 2, 2019

When a strong function is present, the weak function is used when

  • both weak and strong are in a library (.a)
  • the weak function library is listed before the strong function library
  • no other symbol from the strong object file is imported by linker

Example:

decls.h
===========

void func1 ();
void bring_strong_to_me();

main.cc
===========

#include "decls.h"

int main ()
{
#ifdef bringIt
    bring_strong_to_me();
#endif
    func1();
    
    return 0;
}
strong.cc
===========

#include <stdio.h>
#include "decls.h"

void bring_strong_to_me ()
{
}

void func1 ()
{
    printf("strong\n");
}
weak.cc
===========

#include <stdio.h>
#include "decls.h"

void func1 () __attribute__((weak));
void func1 ()
{
    printf("weak\n");
}

Run & results:

#!/bin/sh
set -e
CXX='g++ -Wall -Wextra -Werror'
for i in weak strong; do
    g++ -Wall -Wextra -Werror -c $i.cc -o $i.o
    ar rcs lib$i.a $i.o
done
set -x
$CXX           main.cc     strong.o weak.o -o func && ./func # -> strong
$CXX           main.cc     weak.o strong.o -o func && ./func # -> strong
$CXX           main.cc -L. -lstrong -lweak -o func && ./func # -> strong
$CXX           main.cc -L. -lweak -lstrong -o func && ./func # -> weak
$CXX -DbringIt main.cc -L. -lweak -lstrong -o func && ./func # -> strong

@TD-er
Copy link
Contributor

TD-er commented Nov 2, 2019

Thanks.
That's a very clear example.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 2, 2019

Have you seen a decrease in binary size with this PR ?
If not, maybe the linking order in PIO has to be examined.

@TD-er
Copy link
Contributor

TD-er commented Nov 2, 2019

Have not yet tried this PR, since I have not been near a compile PC today.

@TD-er
Copy link
Contributor

TD-er commented Nov 3, 2019

I did test it and it does save about 20k in build size (using SPIFFS)
Just updated one of my nodes and it works just fine.

@devyte
Copy link
Collaborator

devyte commented Nov 6, 2019

@TD-er please retest. If ok, this can be merged now for 2.6.0.

@TD-er
Copy link
Contributor

TD-er commented Nov 6, 2019

@TD-er please retest. If ok, this can be merged now for 2.6.0.

I will be back home Thursday evening so I cannot test it before.
Maybe @Jason2866 can test it also with Tasmota?

@devyte devyte merged commit 6f7eb28 into esp8266:master Nov 7, 2019
@Jason2866
Copy link
Contributor

We dont use SPIFFS nor LittleFS in Tasmota

@Jason2866
Copy link
Contributor

A short test sketch with SPIFFS worked. Only SPIFFS linked
image

@Jason2866
Copy link
Contributor

Time to release 2.6.0 :-) Tasmota does work reliable with latest master from tonight.
We have best experience using SDK22x_191024 with Tasmota.
With SDK22x_190703 there are a few devices (not many!) which have wifi reconnects
This devices do work too when using SDK22x_191024!

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 7, 2019

We are going to release very soon now, probably this week.
Thanks for the report on the Nonos SDK version.
I'm now wondering if default fw should be 1910 or 1907.

@d-a-v d-a-v deleted the weakfsend branch November 7, 2019 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LittleFS library linked when not used
4 participants