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

Reduce esp_http_client memory usage (IDFGH-3921) #5814

Closed
boarchuz opened this issue Aug 31, 2020 · 3 comments
Closed

Reduce esp_http_client memory usage (IDFGH-3921) #5814

boarchuz opened this issue Aug 31, 2020 · 3 comments
Labels
Type: Feature Request Feature request for IDF

Comments

@boarchuz
Copy link
Contributor

A couple small requests to reduce esp_http_client memory:

  1. Share a buffer for the request and response. Currently they allocate memory separately, yet (as far as I can tell) are never required simultaneously.

    (client->request->buffer->data = malloc(client->buffer_size_tx)) &&
    (client->response->buffer->data = malloc(client->buffer_size_rx))

  2. Expose a function to flush the response. When using the stream method, response data can be processed efficiently in the event callback, however a large-ish buffer is still required simply to discard data from the internal buffer (via repeated esp_http_client_read/esp_http_client_read_response) in order to trigger these callbacks. Something like this would eliminate the need for this additional buffer:

int esp_http_client_flush_response(esp_http_client_handle_t client) {
    int total_len = 0;
    int read_len;
    while(!esp_http_client_is_complete_data_received(client)) {
        read_len = esp_http_client_get_data(client);
        if(read_len < 0) {
            return -1;
        }
        total_len += read_len;
    }
    return total_len;
}

I would submit a PR myself but I'm not 100% sure on the first point.

@boarchuz boarchuz added the Type: Feature Request Feature request for IDF label Aug 31, 2020
@github-actions github-actions bot changed the title Reduce esp_http_client memory usage Reduce esp_http_client memory usage (IDFGH-3921) Aug 31, 2020
@Alvin1Zhang
Copy link
Collaborator

Thanks for raising this feature request.

@shubhamkulkarni97
Copy link
Contributor

Hello @boarchuz,

Thanks for raising this feature request.

esp_http_client is a heavily used component and making change to response and request buffers is a big change. We also think this change can unknowingly break some of the features provided by esp_http_client.

Moreover, there is a user configuration provided in esp_http_client_config_t to configure size of Tx and Rx buffers. You can use this configs to minimise memory usage.

Also, can you share your use case for which you need this feature?

If this change needs to added, we need to evaluate its working for each use case where esp_http_client is used.

Implementation of esp_http_client_flush_response looks good to me. You may go for a PR if you would like, or else I'll add this change internally.

Thanks,
Shubham

@boarchuz
Copy link
Contributor Author

boarchuz commented Sep 9, 2020

Hi @shubhamkulkarni97

There's no particular use case, I just noticed it might be an easy way to free up 512b+ with every esp_http_client. It's not worth creating potential headaches over though.
Thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

3 participants