-
Notifications
You must be signed in to change notification settings - Fork 22
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
Collection of fixes for WLED project #11
Conversation
remove extra #endif that should have been dropped as part of me-no-dev@59066bd
* Added method to access clients of AsyncWebSocket * me-no-dev#571 * Conflict with crypto library
* Fix compile warnings * first test * Add regex path * Add simple example * add support for esp8266 * Update AsyncJson.h
…CONNECT (me-no-dev#559) Signed-off-by: Arthur Frederico Neves <arthurfred.neves@gmail.com>
* Add methods to determine Average Queue length
* Change compiler error to runtime error. Add compiler warining when a ASYNCWEBSERVER_REGEX function is used, but not enabled * Update docs for usage in Arduino IDE * Update docs for platform.local.txt
…wing messageQueue (me-no-dev#621) * Prevent tcp/wifi DOS lockup by preventing number of messages in queue, drop otherwise * Define (renamed) MAX_SSE_Clients
Changes to be committed: modified: src/WebHandlerImpl.h
…e-no-dev#999) * Fix build when using latest arduino-esp32 master due to IDF update espressif/arduino-esp32@a618fc1 * Fix build when using WebSockets
Upgrade the list structure to support quick insertion, and fix a use-after-free in _removeNotInterestingHeaders. This bug has been reported and "fixed" many times upstream, but several of those fixes seem to be bad. See me-no-dev/ESPAsyncWebServer me-no-dev#951, me-no-dev#837
This reduces the total number of allocations required for every request. Also includes a pre-allocation optimization to AsyncWebServerResponse::_assembleHead and some more migration to PROGMEM strings.
Allow the packet buffer to persist, so we can support partial writes in the event that the entire packet cannot be written at once. Includes some heuristics on ESP8266es to ensure that there's enough RAM for the heap and outgoing packet buffers.
This reduces the number of allocations and copies required.
This speeds up our case as we don't need to zero the memory first.
In a misguided attempt to work around badly written clients, AsyncWebSocketClient::_onData was trying to restore the value of the byte one past the end of the packet buffer. Unfortunately, that value might have been *correctly* changed by the callback, such as when it's storing heap metadata. This seemed to be causing of websocket-related crashes in WLED on ESP8266.
Move a few long strings into PROGMEM.
There sure are a lot of long fixed strings.
Also lets us accept PROGMEM for function inputs.
Replace the hand-rolled buffer management logic with an RAII buffer class based on std::shared_ptr. This simplifies memory management and seems to fix a memory leak, while also offering a clean move semantic for clients. The previous API is preserved through a compatibility object; it is flagged as deprecated as the old API required a specialized allocator, which is now unneccessary, so for compatibility reasons an extra heap allocation is used.
Embarrassingly, also included is a metadata patch changing the name to 'ESPAsyncWebServerWLED', updating the version number to 2.1, and listing myself as maintainer. Sorry for the fork, I wasn't sure if you'd have an opportunity to review this work anytime soon. Would you like me to revert those changes? |
Even though I do not understand half of it I do understand that you did an exceptional work. Without any change in WLED code this frees about 5kB of RAM on ESP8266 alone which is outstanding in itself. I am curious, though, why are all those shell scripts needed and how do we change those deprecated methods like @Aircoookie if I understand correctly, your fork was intended primarily to prevent serving |
I merged the changes from the upstream web server repo, which had a lot of CI script work done since the fork. I didn't bother filtering their commits, it seemed to make more sense to be a strictly downstream version. Sadly those scripts have also bit-rotted and no longer work. I'd looked at them briefly - I believe me-no-dev is or was employed by espressif themselves, and some of the espressif repos have newer versions of those scripts -- but they haven't been my priority.
I've got a patch for WLED to eliminate the extra allocation and indirection already in my fork, though it needs to be updated to the final object model. Once the web server changes are accepted (one way or the other) I'll send a PR there. |
As referenced in Aircoookie/WLED#3784 , this is the collection of upstream PRs and bugfixes I've been working on to try and improve performance and stability, particulary for ESP8266es.
Highlights include: