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

Questions about concurrency and memory #79

Open
simap opened this issue Oct 20, 2019 · 2 comments
Open

Questions about concurrency and memory #79

simap opened this issue Oct 20, 2019 · 2 comments

Comments

@simap
Copy link

simap commented Oct 20, 2019

Hi, and thank you for this fork, along with some of @jkent's cmake stuff I'm up and running!

I'm wondering about concurrency and access to the sendBuf.

I noticed some concurrency lock stuff around websocket broadcast, and it looks like the other sending functions don't have that, do they assume to only be called from inside a cgi handler?

Also poking around a bit I noticed the static send buffer changes. It can consume quite a lot of memory (maxConnections * HTTPD_MAX_SENDBUFF_LEN), but I like that it's not going to fragment memory.

If responses are assumed to be composed in callbacks only, is it possible for more than 1 sendBuf to be written to simultaneously from the server task? It doesn't look like there's an intermediate action that could cause writes to sendBuf that don't also get flushed.

It looks like every write to the sendBuf is protected by httpdPlatLock (even the async websocket broadcast). It looks like it is fully synchronous, and you won't have other tasks trying to mutate it simultaneously).

I'm thinking about trying a single static sendbuf per HttpdInstance.

Does this sound like it could work? Am I missing something?

@simap
Copy link
Author

simap commented Oct 20, 2019

As a quick test/hack, I replaced all sendBuff and sendBuffLen references with a global. To make sure, I added a check to make sure when sendBuffLen is set to zero on the start that it is never nonzero, and it seems to work (ran a bunch of concurrent simultaneous downloads and the example test suite). Cuts per-connection memory down to 1120 (1024 of which comes from HTTPD_MAX_HEAD_LEN).

@marbalon
Copy link

@simap I've just checked your idea and tested a little with two browsers: one was loading OTA firmware and the second with configuration/websockets/cgi and everything seems to works fine. I going to use it for a while to verify everything if this is stable solution - thanks!

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

No branches or pull requests

2 participants