Skip to content
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

[WString] Reduce build size by implementing flash string calls in .cpp #8106

Merged
merged 2 commits into from
Sep 4, 2021

Conversation

TD-er
Copy link
Contributor

@TD-er TD-er commented Jun 7, 2021

A function called with a flash string, which only has an implementation with const String& as argument will be compiled as if it is called with a String constructor wrapped around it.

For example this implementation in the .h file:

bool startsWith(const __FlashStringHelper *prefix) const {
            return this->startsWith(String(prefix));
}

This is completely useless as the compiler will generate exactly the same code with or without this function implementation in the .h file.
However if we move the implementation to the .cpp file, this conversion to String is only added once in the compiled binary.
In my own project I already managed to shrink the largest (ESP32) build by more than 70k in size (!!) by just adding extra function calls with the conversion in the .cpp file.
This PR is just a simple optimisation which already shrinks a very small build of my project by almost 3k in build size. (custom_beta_ESP8266_4M1M PIO env of ESPEasy)

Flash: [========  ]  82.5% (used 862137 bytes from 1044464 bytes)
Flash: [========  ]  82.3% (used 859545 bytes from 1044464 bytes)

Larger builds may benefit even more.

A function called with a flash string, which only has an implementation with `const String&` as argument will be compiled as if it is called with a `String` constructor wrapped around it.

For example this implementation in the .h file:
```c++
bool startsWith(const __FlashStringHelper *prefix) const {
            return this->startsWith(String(prefix));
}
```

This is completely useless as the compiler will generate exactly the same code with or without this function implementation in the .h file.
However if we move the implementation to the .cpp file, this conversion to `String` is only added once in the compiled binary.
In my own project I already managed to shrink the largest (ESP32) build by more than 70k in size (!!) by just adding extra function calls with the conversion in the .cpp file.
This PR is just a simple optimisation which already shrinks a very small build of my project by almost 3k in build size.  (custom_beta_ESP8266_4M1M PIO env of ESPEasy)

```
Flash: [========  ]  82.5% (used 862137 bytes from 1044464 bytes)
Flash: [========  ]  82.3% (used 859545 bytes from 1044464 bytes)
```

Larger builds may benefit even more.
@mcspr
Copy link
Collaborator

mcspr commented Jun 8, 2021

I think it's also worth to mention strncmp_P, which could be used for const char* & FlashStringHelper, instead of String(...) conversion? Leaving basic strncmp for the String.
(and since we already assume that const char* could be PSTR(...) with other const char methods)

@TD-er
Copy link
Contributor Author

TD-er commented Jun 8, 2021

Yep for those that can accept const char* that would be an option for sure as you don't need to copy the data from flash if not needed.
Would not make the build smaller, but the memory consumption would benefit from it.

Especially the webserver part would benefit from a similar approach as I have seen that sending flash strings directly to the webserver would make it incredible fast, but also highly unstable when running low on free memory. So if it doesn't need to copy flash strings to memory first but serve it directly from flash string (without the need to copy internally).
And I guess there are other areas too where it could be very useful to have such optimizations.

@earlephilhower
Copy link
Collaborator

Very nice idea! I think your app might be the single largest user of Strings in the world, so most others won't see quite so dramatic a change, but this still seems like a great addition once the 3.0.x bugfixes are in and released.

@earlephilhower earlephilhower added this to the 3.1 milestone Jun 10, 2021
@earlephilhower earlephilhower self-requested a review June 10, 2021 15:10
@TD-er
Copy link
Contributor Author

TD-er commented Jun 10, 2021

[...] I think your app might be the single largest user of Strings in the world [...]

:)
Could be. The largest ESP32 build just passed the 2M build size.

@d-a-v d-a-v added the alpha included in alpha release label Jun 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha included in alpha release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants