-
Notifications
You must be signed in to change notification settings - Fork 3
request->send(200, textPlainStr, jsonChartDataCharStr); - Without using String Class - to save heap #6
Comments
Hi @salasidis I don't know if you're using something different, but as I test here, the HEAP is not
Using Arduino String
The current code is already using If you're interested, I'll post the |
If you look at the top of your example
You can see that the max heap starts at 7.4k Goes to 48k with the String reserve Then goes to 82k when the send occurs. So the max heap is increasing. I noticed sometimes it is only 2x and not 3x, but did not uncover when that is the case. The heap is recovered after the code runs, but the problem is that the program will crash if a large enough string is passed that the max heap hits the 430-450k mark. In my other example, If passing a c string, then the string has to first be created, so that uses Heap RAM (same as having created the string in the first place). In my case, I am sending sensor data, and at 3 months worth, my heap grows to a max of 374k (basically another 60k, and I would expect crashes). However, the program is only using about 60-80k of total heap space when not sending data via the library. My goal is to be able to send a 2Mb string (2 yrs of data) that is stored in SDRAM. In order to be able to do that, any string manipulations would have to avoid arduino strings, and do all the concatenating etc in place on the passed string (as mentioned though, the string buffer would have to be oversized enough to be able to take the added data) |
Ah, I see your point now. The HEAP must expand because of the mandatory copy as mentioned here
Private var to store the
|
String _content; |
The copy is here, to store to private var
Portenta_H7_AsyncWebServer/src/Portenta_H7_AsyncWebResponses.cpp
Lines 255 to 258 in 8297aa2
AsyncBasicResponse::AsyncBasicResponse(int code, const String& contentType, const String& content) | |
{ | |
_code = code; | |
_content = content; |
request->send(200, "image/svg+xml", out);
We could use other way to avoid copy (such as pointer, reference, etc), but we can not know if the original String out
is still there when needed !!! Therefore, copy to be sure.
If you can be sure the original For example request->sendConstantString(200, "image/svg+xml", `out`); |
A little bit odd here, why you have to send 2MBytes (2 yrs of data) in one String ??? Why can't you divide it to many smaller pieces, or to send it more frequently ??? MPUs, with very limited resource, have to be used / treated very carefully and differently from normal PCs. |
It would be best if there was a second function that could take a C string - that way the string can be sent straight from SDRAM. It would save a lot of memory, both for the sending of data, as well as the loading of the original web page (my web page is 70k long - after offloading a bunch of stuff to CDN). I create the web page in an SDRAM string, and after my web page load, the heap goes to 200k in my case. I will unload some of the constant scripts to CDN as well - so this one is manageable, but for others where an even larger web page is created, or if CDN is not possible, then the proposed mod would help them as well. In the data example, the sensor data is stored in the SD card. There are many sensors, and each one may have 2Mb of data. The web page received the data for making custom graphs. It is possible to break up the data into 8 chunks, but that would require 8 separate accesses to the SD card, with 8 file seeks to the correct positions ... Then the data has to be sent to the graphing widget piecemeal, which also takes time. It would all result in creating a graph that is likely 3-4x slower than doing it in one shot. I tried to follow the sequence of the library from the send on out, and I think a c string would be possible, and the only manipulation that was required was adding I believe a header? and a footer (? terminology). Both of those would be done with an in place shift copy of the original string, then copy the header (without the trailing '\0', followed by a strcat of the footer. I think it would be reasonably fast, and would definitely not take any extra memory, except for the small header and footer strings. What do you think? Robert |
This is where the real problem is. The response must have, besides So it's complex, but not impossible, to use original You also have to know ahead the I don't know if it's worth the time to do this, while the simpler solution is to shorten the original |
Another possible way to do is to create a function sendRawString(), where you can
The function just keep the original |
Hi @salasidis After some more serious thinking, I'm be sure this can be done, but I'm sorry I don't have the time and resource to do this for a special use-case. I'd appreciate if you can modify the library to adapt it to your use-case, then make a PR. I'm closing the issue now as Good Luck, |
Thanks for looking into it. (not sure if this is viewed after the issue has been closed - but here goes) I tried single stepping though the library using a j-Link debugger, but ? because of compiler code optimizations ? it does not seem to follow a pattern that I would have expected. As I have no experience writing, or modifying libraries, would it be possible to provide some pointers as to what parts of the library would need to be modified. My best guess as to how to approach this would be to overload the send (and the functions that are called by it) to accept a c string. In AsyncBasicResponse::_respond and AsyncBasicResponse::AsyncBasicResponse Am I way off base, that the only changes required would be in the 2 functions above? Any help would be appreciated. Thanks again. |
Basically correct. You have to try and modify step-by-step, then you'll be OK. I certainly can help on the way, if necessary. I'll be happy to have a new |
Thanks for the support. I will go the easiest route I know, which is to add a variable in the AsyncBasicResponse class called char * _clientCstr. In the routines mentioned above, I will simply check if _clientCstr is null or not, and use it, or the regular _client arduino String. Should require the most minimum of extra code hopefully. I will keep you posted. Robert |
Tried to create a pull request (first time), but it created a fork - not sure if that is what I was supposed to do. In any case, the finished code is here https://github.com/salasidis/Portenta_H7_AsyncWebServer/blob/main/README.md As per the Readme file - I am getting 66k instead of 374k max heap space in my application. I should havbe no problem sending 2M files, or web pages larger than the main RAM Since I create the web page once, and reuse, I make a second copy of the web page, so that when the send happens (which messes wit the actual string), I simply recopy C string C string for the next send - fast, easy, and almost no code changes required (none if you don't wish to use the features 0 fully backwards compatible I think)
|
I temporarily created a PR to have an easier look of the files. LGTM. You've written very good code. Can you make some minor changes to match with the current styles
BTW, to create a PR, after you've forked the library and have done all the mods, just press I'm deleting the PR and let you do it. |
Check the RSMOD branch for the modified code. Test / modify as necessary. Then create a new PR |
I see you made all the changes already, thanks. I had a question about the original software. If you look at line 447 and 450 in the AsyncWebResponses.cpp file. The original program sets the _content String to String(). I had decided to do the same with the Cstring, by doing a _contentCstr = '\0'; What that did was that on the last sent packet, there was a '\0' on the string being sent embedded into the file. When I commented it out, the code worked OK (that is why I added all the debug serial prints - to try to see if that was the source of the error. The question is, is the same thing happening in the original code, except the string has not been reclaimed by the heap yet, so the error does not show up??? Does it need to be there ?? |
Don't see that line. Do you mean Portenta_H7_AsyncWebServer/src/Portenta_H7_AsyncWebRequest.cpp Lines 447 to 450 in 8297aa2
|
Could you also add examples to demo the new functions and better |
No should be in In the mod file it is the following Using permalinkPortenta_H7_AsyncWebServer/src/Portenta_H7_AsyncWebResponses.cpp Lines 447 to 448 in 3f11c76
Using code tagsize_t AsyncBasicResponse::_ack(AsyncWebServerRequest *request, size_t len, uint32_t time)
{
...
...
...
if (_contentCstr)
{
_writtenLength += request->client()->write(_contentCstr, available);
//_contentCstr[0] = '\0'; // <----- this gave error described until commented out
}
else
{
_writtenLength += request->client()->write(_content.c_str(), available);
_content = String(); // I<---- had placed it because of this - should this be there ?
}
_state = RESPONSE_WAIT_ACK;
return available;
}
` |
Will try to add the files to the RSMOD examples - will run them first to make sure |
Worked nicely -= went from 111k to 12.7k max heap I uploaded them to my fork, as it sad I had no push access to add them to RSMOD Can you take them from there? https://github.com/salasidis/Portenta_H7_AsyncWebServer/tree/main/examples/Ethernet |
That's the way it must be. Only author / collaborator can modify any branch. You can only make PR.
I can. But it's better you make a PR for everything as you'll use the PR many more times. |
I did a pull request, but I don't know how to merge your changes on my end (it says conflicts - and no idea how to resolve them). So, it did a pull request for the files you already fixed up as well. |
You just either
For example,
then you delete all the markers (
until all markers are deleted, then press Repeat for all 4 files. Then you'll be ready to merge to your GitHub |
Pull Requested the NO SDRAM file. Also, have you had a chance to look at the code below - should I try to see how it works eliminating the line ****
|
Closed as completed via #8 |
As you mentioned in Arduino Forum AsyncWebServer Library - heap memory use
Do you think we can reuse the cString by just changing the address, such as creating a new private pointer and update it according to the change ? For example, change
to
and in
Just an idea, I haven't tested and verified. |
BTW, I just ported your PR into AsyncWebServer_RP2040W library for RP2040W (WiFi) and OK now with the heap The preliminary results are Async_AdvancedWebServer_MemoryIssues_Send_CString on RASPBERRY_PI_PICO_W with RP2040W CYW43439 WiFiUse CString => Max Heap = 43,976 bytes
Use String => Max Heap = 75,240 bytes
|
I thought about it on my end as well. I am not sure how often the header is larger than the first packet, but, since that is the only time. Can create a second string that has the remnant header (could be an arduinovstring, as likelv<1k. Then when relooping into code, check if the remnant header is empty, and if not, make it the new header, and zero length the remnant string. Then can continue running the code as before. Would take only maybe 3-4 lines new code, and no rewrites in the main body. We would then never need to care about having space in the larger string, and would allow the elimination of all the memmove calls. |
Looking at the code, I think we need a combination. Going in order, Section 1 - no memmoves - content is 0 length - no change Section 2 - the content and header all fit in 1 packet- we can just create an Arduino string and send it (the old way) - we lose a packet worth of heap, so not much In Section 3 - the content length is unknown, but the the header is long, so we can do what I mentioned, send the partial header, and create a remnant header string to send in the RESPONSE_CONTENT loop In Section 4 - Header is small, but content could be large - Leave as is - as the header is sent with a little added buffer Section 5 - default - not sure why it would ever get here - to me the other instances seem to cover all the bases. But we would have to create a new header from the old, and proceed as in 3 In RESPONSE_CONTENT before entering if/else statements
I can try all this later this week and let you know |
Sounds good. It's worth a try. Waiting for results form your mods and tests. |
Send a pull request with the mods - not fully tested yet, but works on my main application Non destructive, no extra space needed |
I ran the Cstr example, and it works. Max Heap 12795 - and seems to stay stable |
Your PR is very contagious ;=)), has been spreading to AsyncWebServer_WT32_ETH01 library for WT32_ETH01 (ESP32 + LAN8720A) and helps a big deal to save the heap. The current results are Async_AdvancedWebServer_MemoryIssues_Send_CString on WT32-ETH01Using CString => heap = 120,876 bytes
Using String => heap = 152,088 bytes, around one time data buffer (~31,220 bytes) larger.
|
Haven't seen anything yet. PR forgotten ??? |
Sent it again, if you did not get it, I am doing it wrong :-( Only 2 files changed |
Great. Got it now. Will have a look later today. |
Closed as done in Portenta_H7_AsyncWebServer v1.4.1 Looking forward to receiving many more of your PRs, issue reports, enhancement requests, etc. Best Regards, |
Hi @salasidis Your PRs has just been spreading to AsyncWebServer_Ethernet library for ESP8266 and W5x00 / EMC28J60 Ethernet and helps a big deal to sned bigger data because of heap saving. ESP8266's heap space is much smaller than ESP32's |
Posted by @salasidis as request->send(200, textPlainStr, jsonChartDataCharStr); - Without using String Class - to save heap #2 in Portenta_H7_AsyncTCP library
Is your feature request related to a problem? Please describe.
I have a relatively web page - about 100kBytes. In addition, I need to send large amounts of Json data - about 1Mbyte sensor data via the async web interface.
I save all the data in the SDRAM (the web page as well as the Json data string (created from SD Card logged data).
When responding to the request via
request->send(200, textPlainStr, jsonChartDataCharStr); // jsonChartDataCharStr is a C type char * string
The library converts the (char *) array (JSon or the web page) into a String class variable before sending. This results in large amounts of heap use, and basically I cannot send more than 4 months of telemetry data (whereas the design calls for at least 2 years).
Describe the solution you'd like
I would like to have a request->send call that looks at the variable passed, and if it is a char * leaves it as such, without changing it to a String. Not only will this be faster, but it will also use a LOT less memory
Describe alternatives you've considered
My alternatives of making many calls to retrieve the data in small batches would add needless complexity, and also require multiple accesses to the SD card, which I want to avoid
Additional context
This would not be an issue if heap space was not so critical, but the portenta only has a small finite amount, and this library consumes about 80+% of it in my application in a single function call.
It would be nice if any mallocs could be limited, or at least made to optionally use the SDRAM, and not the main RAM.
Also any string manipulations (adding a header etc) could be done in place - just having to ensure that the passed string is at least a minimum size larger than the contents it holds.
On the default program, I added some code to print the heap size, and I increased the graph array from 50 to 500 points.
On the modified program, I created a C string instead of an Arduino String, and sent the same 500 line graph svg file
There is a word file that shows the outputs, and the outputs demonstrate that if sending an Arduino String, the software requires 2x extra heap space on top of the the actual string size.
If sending a C string, it takes 3x.
My proposal is to have an option (separate call, or modify the current ones), that can use the c string as is, and simply pre and post append to it any headers and footers required, without creating new Arduino Strings in the library
File Async_AdvancedWebServer_MemoryIssues_SendArduinoString.ino
File Async_AdvancedWebServer_MemoryIssues_Send_CString.ino
Heap Data when Sending a 500 line string
Heap Data when Sending a 500 line string (string is 31.2k long).
Max Heap = 7.4k – before executing graph routine
Max Heap = 48.5k – after reserving 40k for out string
Max Heap = 111.2k – after send
When sending am Arduino String, the library requires 2x the String length in extra heap.
The second program modifies the source code, so that instead of creating the out String, I create a C string called cStr. This has a buffer allocated in SDRAM. I then create the cStr in a similar fashion as before, except using strcat etc. (I remove the meta .. refresh – just to make the output cleaner)
When running the code – a 500 line graph string is sent here as well – the string is again 31.2k long
Max Heap = 8.4k – before executing graph routine
Max Heap = 13.5k after creating the string, but before send *** note that there is no heap increase as before
Max Heap = 107.5k – after send
When sending a c string, the web server uses 3x the c string size iin heap space (1x more than before, as it makes a String out of the c string before proceeding
The text was updated successfully, but these errors were encountered: