nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

Reduce malloc/free usage for HTTP responses #205

Closed jessie-murray closed 2 years ago

jessie-murray commented 2 years ago

This pull request makes two changes: the first to avoid copying header strings for http_response when static values are used, and the second to avoid re-allocating the headers array for each header being added to it.

The value of 9 for DEFAULT_HEADERS_ARRAY_SIZE was found with a curl to /PING, which returned 9 headers.

I ran Webdis under valgrind to count the allocations produced by ./basic.py.

Before:

==10== HEAP SUMMARY:
==10==     in use at exit: 22,404 bytes in 233 blocks
==10==   total heap usage: 4,122 allocs, 3,889 frees, 340,545 bytes allocated

After:

==10== HEAP SUMMARY:
==10==     in use at exit: 22,916 bytes in 233 blocks
==10==   total heap usage: 3,000 allocs, 2,767 frees, 281,176 bytes allocated

This reduced the number of malloc calls by over 27%, and free by the same count. The same number of blocks were left allocated after the test and both executions reported 0 memory leaks.

nicolasff commented 2 years ago

This looks great, thanks for adding the new flag.

Since the original test was just with basic.py, I ran the same comparison with ws-tests.py, getting "7,767 allocs, 7,530 frees, 28,872,327 bytes allocated" with the latest release and "7,767 allocs, 7,530 frees, 28,873,671 bytes allocated" with this pull request.

I was surprised to see the same number of allocations, before realizing this made total sense given that ws_handshake_reply builds the response with HTTP/1.1 101 Switching Protocols and not the code in http.c.

With just basic.py, I see the the same improvements as those reported above, "4,123 allocs, 3,890 frees, 341,057 bytes allocated" on 0.1.18 vs "3,000 allocs, 2,767 frees, 281,176 bytes allocated" with this PR.

Well done!