lasselukkari / aWOT

Arduino web server library.
MIT License
283 stars 41 forks source link

Feature Request / Bug On Some Platforms: Reduce memory footprint by wrapping all C strings in `F(...)` #106

Closed EmperorArthur closed 3 years ago

EmperorArthur commented 3 years ago

Hello,

I was recently evaluating your library to replace our custom Arduino HTTP Server implementation. Of all the libraries examined so far, yours is definitely the most polished, and certainly has better routing capabilities than the nested if statements ours uses. Please do not take my suggestions as thinking your library is "bad."

In the process of evaluating your library, I noticed that it uses 1398 bytes of RAM. I believe this is because, despite importing "Arduino.h", the library uses raw C strings for everything. Those strings are always stored in RAM, just sitting there doing nothing but taking up space.

This limits what devices the library can be run on, and/or the amount of data processing that is available when using this library.

Suggested Fix

#include <WString.h> and then use F(<HttpStatus_reasonPhrase>). An alternate approach for some code is to use this repository, with sleight modifications.

EmperorArthur commented 3 years ago

If you are interested, I should be able to submit a pull request to resolve this issue.

lasselukkari commented 3 years ago

Hi.

Most strings that are sent to to client are already using the progmem with the P macro. I have been trying to avoid the dependency to String class. Maybe there is no good reason for that.

Feel free to make a pull request. I will be interesting to see how much additional memory this will save. I will most likely also need to update the CI scripts to support this.

EmperorArthur commented 3 years ago

Thanks for the response. I used this tool to examine what is being stored in RAM, and took a deep dive in the code.

It looks like most of the usage I was seeing was because of the 128 and 1024 buffers. You may want to document that somewhere. Especially since some of us (me mostly) are used to using things like ArduinoStreamUtils.

However, I believe I can save at least 50 bytes of RAM just by wrapping the C strings in m_processMethod with:

#define FC(string_literal) String(F(string_literal)).c_str()
FC("some string")

I am continuing to investigate, and will send a PR soon.

EmperorArthur commented 3 years ago

That macro approach didn't work out as well as I'd like, however standard Arduino F strings were the solution.

PR #109 saves 98 bytes worth of RAM, and actually seems to decrease flash usage by 54 bytes as well. I assume the optimizer is able to remove or combine more code.

Edit: It's important to note that the Response.set(...) function does not like F() strings. That can cause static RAM bloat from normal user code and is costing a few bytes here. However, that should probably be another issue/PR for after this one is resolved.

lasselukkari commented 3 years ago

PR merged.