Skip to content

CivetWeb HTTP Sample - reduce RAM usage #28376

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

Merged

Conversation

KozhinovAlexander
Copy link
Collaborator

Reduce RAM memory usage by the CivetWeb HTTP sample

This PR solves #21179

@KozhinovAlexander
Copy link
Collaborator Author

@erwango Please review and test it.

@tbursztyka
Copy link
Collaborator

add "Fixes #21179" into your commit message

@erwango
Copy link
Member

erwango commented Sep 15, 2020

@Nukersson I'm not able to build samples/net/sockets/civetweb:

/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c: In function 'websocket_client_thread':
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17311:19: error: storage size of 'sa' isn't known
17311 |  struct sigaction sa;
      |                   ^~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17315:18: error: 'SIG_IGN' undeclared (first use in this function)
17315 |  sa.sa_handler = SIG_IGN;
      |                  ^~~~~~~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17315:18: note: each undeclared identifier is reported only once for each function it appears in
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17316:2: warning: implicit declaration of function 'sigaction' [-Wimplicit-function-declaration]
17316 |  sigaction(SIGPIPE, &sa, NULL);
      |  ^~~~~~~~~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17316:12: error: 'SIGPIPE' undeclared (first use in this function); did you mean 'EPIPE'?
17316 |  sigaction(SIGPIPE, &sa, NULL);
      |            ^~~~~~~
      |            EPIPE
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17311:19: warning: unused variable 'sa' [-Wunused-variable]
17311 |  struct sigaction sa;
      |                   ^~

This isn't linked with your PR, nor the board. I'm seeing this also on master and on other board.
I'm using v2.4.0-rc1-118-gdc79b37167 (latest master), and run west update.

Do I miss something ?

@KozhinovAlexander
Copy link
Collaborator Author

KozhinovAlexander commented Sep 15, 2020

@Nukersson I'm not able to build samples/net/sockets/civetweb:

/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c: In function 'websocket_client_thread':
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17311:19: error: storage size of 'sa' isn't known
17311 |  struct sigaction sa;
      |                   ^~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17315:18: error: 'SIG_IGN' undeclared (first use in this function)
17315 |  sa.sa_handler = SIG_IGN;
      |                  ^~~~~~~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17315:18: note: each undeclared identifier is reported only once for each function it appears in
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17316:2: warning: implicit declaration of function 'sigaction' [-Wimplicit-function-declaration]
17316 |  sigaction(SIGPIPE, &sa, NULL);
      |  ^~~~~~~~~
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17316:12: error: 'SIGPIPE' undeclared (first use in this function); did you mean 'EPIPE'?
17316 |  sigaction(SIGPIPE, &sa, NULL);
      |            ^~~~~~~
      |            EPIPE
/local/mcu/zephyrproject/modules/lib/civetweb/src/civetweb.c:17311:19: warning: unused variable 'sa' [-Wunused-variable]
17311 |  struct sigaction sa;
      |                   ^~

This isn't linked with your PR, nor the board. I'm seeing this also on master and on other board.
I'm using v2.4.0-rc1-118-gdc79b37167 (latest master), and run west update.

Do I miss something ?

Thanks for the testing. Yes, you're missing latest civetweb. Just add it to west.yaml file and provide west update. This change comes with my other PR (see #28323). Sorry, I did not expected this error in this case. If you want we can skype - then I will give you more explanations. Please let me know your test result of this PR.

@KozhinovAlexander
Copy link
Collaborator Author

@erwango I've just added updated west.yml file and corresponding PR for it. Thus, compiling this PR should work.

@KozhinovAlexander
Copy link
Collaborator Author

add "Fixes #21179" into your commit message

Done

@erwango
Copy link
Member

erwango commented Sep 15, 2020

I'm not able to test, but I can confirm the reduction of memory consumption.
Before:

Memory region         Used Size  Region Size  %age Used
           FLASH:      241096 B         2 MB     11.50%
             CCM:          0 GB        64 KB      0.00%
            SRAM:      236159 B       192 KB    120.12%
        IDT_LIST:         216 B         2 KB     10.55

After:

Memory region         Used Size  Region Size  %age Used
           FLASH:      125880 B         2 MB      6.00%
             CCM:          0 GB        64 KB      0.00%
            SRAM:       93295 B       192 KB     47.45%
        IDT_LIST:         216 B         2 KB     10.55%

@KozhinovAlexander
Copy link
Collaborator Author

KozhinovAlexander commented Sep 15, 2020

I'm not able to test, but I can confirm the reduction of memory consumption.
Before:

Memory region         Used Size  Region Size  %age Used
           FLASH:      241096 B         2 MB     11.50%
             CCM:          0 GB        64 KB      0.00%
            SRAM:      236159 B       192 KB    120.12%
        IDT_LIST:         216 B         2 KB     10.55

After:

Memory region         Used Size  Region Size  %age Used
           FLASH:      125880 B         2 MB      6.00%
             CCM:          0 GB        64 KB      0.00%
            SRAM:       93295 B       192 KB     47.45%
        IDT_LIST:         216 B         2 KB     10.55%

Under not able do you mean - the server does not starts or you have some other problems?

@erwango
Copy link
Member

erwango commented Sep 15, 2020

Under not able do you mean - the server does not starts or you have some other problems?

Server doesn't start indeed, but I'm at home with limited test possibilities, so I haven't checked much more.
I should be able to test more at the office tomorrow.

@KozhinovAlexander
Copy link
Collaborator Author

KozhinovAlexander commented Sep 15, 2020

Under not able do you mean - the server does not starts or you have some other problems?

Server doesn't start indeed, but I'm at home with limited test possibilities, so I haven't checked much more.
I should be able to test more at the office tomorrow.

Probably following advice would be helpful for you:

My observations shows, that civetweb server samples needed too much time to start up (sometimes about > 30 sec.). This behavior can be seen on current master's branch civetweb implementation and also on implementation of this PR.
Following thing can help: Ping the IP of your server. And as soon as the response comes - go to the server over your web browser.

P.S.: The fastest browser on my MacBook was Chrome. The Safari needed much more time to load pages from embedded CivetWeb server on my NUCLEO-H745ZI-Q board running @480MHz. Mozilla were not properly tested by me.

@erwango
Copy link
Member

erwango commented Sep 16, 2020

Under not able do you mean - the server does not starts or you have some other problems?

Server doesn't start indeed, but I'm at home with limited test possibilities, so I haven't checked much more.
I should be able to test more at the office tomorrow.

Probably following advice would be helpful for you:

My observations shows, that civetweb server samples needed too much time to start up (sometimes about > 30 sec.). This behavior can be seen on current master's branch civetweb implementation and also on implementation of this PR.
Following thing can help: Ping the IP of your server. And as soon as the response comes - go to the server over your web browser.

P.S.: The fastest browser on my MacBook was Chrome. The Safari needed much more time to load pages from embedded CivetWeb server on my NUCLEO-H745ZI-Q board running @480MHz. Mozilla were not properly tested by me.

Had some more tries today, but can't connect to the board anyhow. I don't blame the samples though, as heavily controlled corporate machines might now help.

@KozhinovAlexander
Copy link
Collaborator Author

Under not able do you mean - the server does not starts or you have some other problems?

Server doesn't start indeed, but I'm at home with limited test possibilities, so I haven't checked much more.
I should be able to test more at the office tomorrow.

Probably following advice would be helpful for you:
My observations shows, that civetweb server samples needed too much time to start up (sometimes about > 30 sec.). This behavior can be seen on current master's branch civetweb implementation and also on implementation of this PR.
Following thing can help: Ping the IP of your server. And as soon as the response comes - go to the server over your web browser.
P.S.: The fastest browser on my MacBook was Chrome. The Safari needed much more time to load pages from embedded CivetWeb server on my NUCLEO-H745ZI-Q board running @480MHz. Mozilla were not properly tested by me.

Had some more tries today, but can't connect to the board anyhow. I don't blame the samples though, as heavily controlled corporate machines might now help.

Have you tested this PR on dev board connected to your local machine's Eth port, s.t. you can provide proper IPv4 to your board? If you want I can give you support over TeamViewer or so. Or we can Skype anytime tomorrow or today evening.

P.S.: My board runs connected to my local network router.

@KozhinovAlexander
Copy link
Collaborator Author

@lochej Can you test the sample also please?

@lochej
Copy link
Collaborator

lochej commented Sep 16, 2020

@lochej Can you test the sample also please?

@Nukersson I performed a west update then fetched your branch locally to test.
Connected my board directly to my PC: Nucleo_H743ZI (no st-link/custom) + Killer ethernet E4200

I changed the IP config to accomodate for my static IP local ethernet link settings:

CONFIG_NET_CONFIG_MY_IPV4_ADDR="192.0.2.1"
CONFIG_NET_CONFIG_MY_IPV4_NETMASK="255.255.255.0"
CONFIG_NET_CONFIG_MY_IPV4_GW="192.0.2.2"
CONFIG_NET_CONFIG_PEER_IPV4_ADDR="192.0.2.2"

So far so good, the sample runs fine and displays the CivetWeb sample for each routes correctly:
image
image
image

Also network performance is pretty good:

ping -i 0.2 192.0.2.1 -c 30
--- 192.0.2.1 ping statistics ---
30 packets transmitted, 30 received, 0% packet loss, time 5877ms
rtt min/avg/max/mdev = 0.535/0.894/1.145/0.128 ms

Edit:
CivetServer is up and accessible 5ish seconds after releasing reset to the board. In fact, I press the refresh button of Chrome and release the reset at the same time. Note that I'm connected directly to the board

@erwango
Copy link
Member

erwango commented Sep 17, 2020

@Nukersson, I finally made it on nucleo_f429zi.
Runs successfully! And perf are not that bad neither (for a lower end platform vs H7)

--- 10.0.0.111 ping statistics ---
30 packets transmitted, 30 received, 0% packet loss, time 29382ms
rtt min/avg/max/mdev = 0.402/0.563/0.729/0.085 ms

Copy link
Collaborator

@lochej lochej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Works on Nucleo-H743ZI

@erwango erwango added this to the v2.5.0 milestone Sep 17, 2020
@erwango
Copy link
Member

erwango commented Sep 17, 2020

@Nukersson Thanks to this PR. I'm adding the V2.5.0 milestone as this doesn't exactly fit the criteria to be merged in V2.4.0 release stabilization period (not strictly a "bug" fix).
It will be integrated at V2.5.0 merge window opening

@KozhinovAlexander
Copy link
Collaborator Author

@Nukersson Thanks to this PR. I'm adding the V2.5.0 milestone as this doesn't exactly fit the criteria to be merged in V2.4.0 release stabilization period (not strictly a "bug" fix).
It will be integrated at V2.5.0 merge window opening

I am very happy. Thank you for testing it also. Yes, I am agree - it should be merged into v2.5.

@KozhinovAlexander
Copy link
Collaborator Author

@Nukersson, I finally made it on nucleo_f429zi.
Runs successfully! And perf are not that bad neither (for a lower end platform vs H7)

--- 10.0.0.111 ping statistics ---
30 packets transmitted, 30 received, 0% packet loss, time 29382ms
rtt min/avg/max/mdev = 0.402/0.563/0.729/0.085 ms

���@erwango Have you observed ping between system startup and ping answer on your F4 board? How much time does it took, if yes?

@erwango
Copy link
Member

erwango commented Sep 18, 2020

@erwango Have you observed ping between system startup and ping answer on your F4 board? How much time does it took, if yes?

No, I haven't. If finding spare time today, I'll have a new try, but do not rely on it.

@KozhinovAlexander KozhinovAlexander force-pushed the civetweb_reduce_ram_usage branch 4 times, most recently from 19264aa to 13cce24 Compare September 24, 2020 11:51
@KozhinovAlexander
Copy link
Collaborator Author

 @pfalcon @tbursztyka Please provide review.

@KozhinovAlexander KozhinovAlexander force-pushed the civetweb_reduce_ram_usage branch 2 times, most recently from 996a9eb to fedc4c2 Compare September 29, 2020 07:59
@KozhinovAlexander KozhinovAlexander force-pushed the civetweb_reduce_ram_usage branch 2 times, most recently from abef31b to 5fa3152 Compare September 29, 2020 11:44
adjust configuration so that civetweb uses less memory
fixes zephyrproject-rtos#21179

Signed-off-by: Alexander Kozhinov <[email protected]>
@jukkar jukkar merged commit c28ffbb into zephyrproject-rtos:master Sep 30, 2020
@KozhinovAlexander KozhinovAlexander deleted the civetweb_reduce_ram_usage branch September 30, 2020 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants