Sitelet https://github.com/ESP32Async/ESPAsyncWebServer/pull/480
Skip to content

AsyncEventSource queue management - #480

Open
willmmiles wants to merge 3 commits into
mainfrom
asynceventsource-perf
Open

willmmiles wants to merge 3 commits into
mainfrom
asynceventsource-perf

Conversation

@willmmiles

Copy link
Copy Markdown

Some fixes and performance improvements to AsyncEventSource that fell out during AsyncTCP regression testing.

Track when we've filled the underlying socket and stop until we receive
an ack.  Reduces CPU usage blindly retrying when the buffer is full.

Also fixes a correctness issue where a _runQueue which partially sends a
message will still attempt to send the next message, which will tear if
an ack arrives in between writes.
Drop redundant space check prior to attempting data send.  add() has to
check for available space internally, interlocked with the TCP layer;
the additional check just wastes time.
As we are committed to ASYNC_WRITE_FLAG_COPY, we can release written
event messages as soon as they are accepted by the TCP socket.

Originally implemented in:
- esphome#41
- mathieucarbou/ESPAsyncWebServer#92

Reverted by 43e0b5c in the hopes of supporting zero-copy TCP writes,
but (as noted in that commit!) they are not supported on any Espressif
platform, so the additional complexity buys no benefit.
@mathieucarbou

Copy link
Copy Markdown
Member

Thanks @willmmiles !
Did you run some tests or not yet ? I can if think I need to :-)

@willmmiles

Copy link
Copy Markdown
Author

Thanks @willmmiles ! Did you run some tests or not yet ? I can if think I need to :-)

I've done a fair amount of stress testing, this was one of the performance regression testing targets for the recent AsyncTCP work. I can't say I've tried every corner case though; if you know where the trouble spots are likely to be, please give those corners a poke!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants