[Cleanup] Reduce build size + clean up loose ends #3655
Conversation
By stripping of an extra parameter, the build size is reduced quite a lot.
Wrapping functions to convert `LabelType::Enum` into String into separate function to reduce build size
Remove some code duplication by streaming JSON from PROGMEM arrays of `LabelType::Enum`. This makes the code more readable and also (slightly) reduces build size. (at least memory usage when serving JSON)
Each call to a function with a flash string, expecting `String` argument is adding a `String()` wrapper increasing build size.
Less memory allocations and streaming flash strings as such without converting to `String`
Macros including function calls with lots of variables tend to add quite a lot to the build size.
|
OK, I'll stop now as it gets larger and larger.... The "ESP8266 display" build is almost 37k smaller with this PR. Still for the minimal OTA builds, roughly 5 - 8k must be stripped to make them fit again. (already made roughly 16k smaller for those builds) The biggest change here is to have less conversions from flash string to String in the code. void foo(const String& bar) {...}
foo(F("bar")); // Compiler: foo(String(F("bar"))); This is also done when returning a function, or declaring a variable: String options[2] = {F("1"), F("2") };
// Compiler:
// String options[2] = { String(F("1")), String(F("2")) };
String MyTypeToString(int value) {
switch (value) {
case 1: return F("one"); // Compiler: return String(F("one"));
case 2: return F("two"); // Compiler: return String(F("two"));
}
}By returning a Flash String type for those functions, you only need a single conversion to a String when assigning it. |
To reduce build size as those do not have to be converted into a String type.
Saves quite a bit from the binary size.
Should be tested, as the numbers now no longer are wrapped in quotes.
|
OK, could not leave it.... FHEM HTTP controller should be tested as the generated JSON now slightly differs from the old implementation. So if anyone using FHEM could test it, please do! |
This gets rid of another rather big chunk of (generated) code which does add up to the binary size.
|
@clumsy-stefan Added your suggestions and got rid of another rather big chunk of code by removing the |
|
Warning! I made an error in the htmlEscapeChar function, so streaming rules to the browser may also strip out characters which will then be lost when saving the rules. So wait a moment for a fix for this.!!! |
|
And it should now be fixed. |
|
Oh, ok... just updated all nodes ;) so I'll do a new version and update again... Not sure about the AddLog (or at least I can't remember ;) but sounds good!! One other code duplication was about the login, it seems it accepts basic-auth as well as the HTTP form-login.. |
I had created a macro, a long time ago, which does check for the loglevelactive before actually creating the log string. The drawback of this approach is that constructed strings (or functions generating a |
|
New sizes: 1-2k improvment again. Main-Page link works again! |
... And hopefully this may help in making Linux and Windows builds create more equal sized binaries.
Another few bytes saved with latest commit! |
|
What I also found is, that the new commits in this PR increase memory usage. You can see in the graphs below, that since updating (around 12:00) Free memory is significantly lower (around 2k):
|
|
Some parts of the code check the max. free memory and adjust the logging/cache/web and controller queue behavior based on this. But on the other hand, scanning WiFi may still be active? Or perhaps the sending of data to a controller is not flushing? |
|
I did not change anything in the config, so wifi scanning was active before and after the update. Also I did not notice any change in the controller, but I only use FHEM all others are not even compiled in. Could it be, that things which are not anymore in the build (and reduce the size) need to be "calculated" at runtime and therefore use more memory? |
|
Not that I can think of. So all changes should mainly be beneficial as there is:
Since a conversion to |
|
The latest build ran more or less stable for the last 12 hours. Only Problem I encountered was that the lower memory combined with bad WiFi receiption, a lot of interaction on the web pages or a lot of tasts/devices seems to generate exceptions from time to time.... But this seems not critical to me. I'd vote in favor to include this PR as quickly as possible as the improvements are really significant especially for the small builds! Thanks for the great work done here!! |
|
Found some more possibilities, but those require changes in the Arduino I will later this evening merge it. |
| @@ -124,7 +124,7 @@ bool do_process_c004_delay_queue(int controller_number, const C004_queue_element | |||
| String postStr = do_create_http_request( | |||
| hostName, F("POST"), | |||
| F("/update"), // uri | |||
| F(""), // auth_header | |||
| EMPTY_STRING, // auth_header | |||
There was a problem hiding this comment.
Is this a generic improvement, simply replace all F("") by EMPTY_STRING ? Or does it require a __FlashStringHelper reference?
(If so then I'll start updating that in my work in progress 😉)
There was a problem hiding this comment.
Nope, it is slightly more complex :)
For those functions that return a String object, this is an optimization as you don't need to create a String object, but simply reference a static const existing one.
But it is a bigger reduction in size if you can create functions that return a flash string instead of having code for every assignment of a flash string to a String object.
e.g. a typical toString function:
String toString(myEnumClassType enumValue) {
switch (enumValue){
case myEnumClassType::bla:
return F("bla");
}
}This return will effectively generate return String(F("bla")); for every flash string return calls.
By returning a flash string type, you only cast it to String after the function is called. (or not at all if it can be kept as a flash string)
For example the webserver markup function calls that now also accept a flash string array, which only holds a pointer instead of allocating memory for the string only to dump it to the TXBuffer object, which also can handle flash strings without conversion.
That's the true change of this PR.
Unfortunately the free memory was decreased significantly in my case. :-(
|
|
Do you have "Periodical WiFi scan" enabled? |
|
No, it's disabled. |
|
Do you use mDNS in your builds? (to get a |
|
I have never needed it so unless it's a default option, the answer is NO. I did not enable it (and I can't find it anywhere). Edit - regarding to mDNS, I have found it should be enabled by #define FEATURE_MDNS. So I can confirm this definition does not exist in my Custom.h file. |
|
OK, so we can eliminate that from the list of possible memory hogs. |
|
|
|
And the queue depth of the controllers? |
|
Everything looks OK with controllers, intervals are 30s, 15s, 60s in plugins 6, 7, 8 and no visible delays encountered.
|
|
I have temporarily disabled the Communication - TSOP4838 plugin, then the Free RAM increased to 10000 - 12000... |
@giig1967g Should for sure also test the GPIO code as I tried to remove quite a bit of code duplication (quite a reduction of a few k in build size)
I really hope to not have changed the behavior.