esp-protocols icon indicating copy to clipboard operation
esp-protocols copied to clipboard

Add memory_type Support for Flexible Memory Allocation in WebSocket Client to permit other memory type caps to be used (IDFGH-13127)

Open filzek opened this issue 1 year ago • 8 comments

Enable Usage of External or Specific Memory Types in WebSocket

Feature: Added a memory_type directive to esp_websocket_client_config_t to allow usage of various memory types in heap_caps_malloc and heap_caps_calloc.

Default Handling: If memory_type is not defined or is invalid, MALLOC_CAP_DEFAULT is used. HTTP Auth Update: Updated http_auth_basic function to use the specified memory region.

All relevant functions have been updated to utilize the new memory_type configuration.

Now can set any memory CAPS region such as MALLOC_CAP_SPIRAM and others.

filzek avatar Jun 03 '24 21:06 filzek

+1 Maybe allocate websocket task stack also from selected RAM by using xTaskCreateWithCaps?

erkia avatar Jun 25 '24 04:06 erkia

@filzek Thank you for your contribution. However, the changes are causing compilation issues in our CI pipeline. Please refer to the details here: https://github.com/espressif/esp-protocols/actions/runs/9708563633/job/26876119855?pr=586

gabsuren avatar Jul 01 '24 07:07 gabsuren

@filzek Thank you for your contribution. However, the changes are causing compilation issues in our CI pipeline. Please refer to the details here: https://github.com/espressif/esp-protocols/actions/runs/9708563633/job/26876119855?pr=586

changes made to the original file as requested!

filzek avatar Jul 04 '24 19:07 filzek

I will provide corrections for the SDK version to build the SDK. Some memory types are strictly dependent on the SDK version, such as MALLOC_CAP_TCM, which is only available after version 5.2. I will check all kinds of memory for each SDK version and provide #ifdef directives for those types.

filzek avatar Jul 10 '24 12:07 filzek

@filzek Thank you for the update. However, there are still breaking changes in the CI due to the modifications.

https://github.com/espressif/esp-protocols/actions/runs/9798927333/job/27229816060?pr=586

gabsuren avatar Jul 24 '24 10:07 gabsuren

@erkia

xTaskCreateWithCaps

We're currently testing the option to choose the core on which tasks will run, in addition to specifying the memory region. We believe this could enhance speed and consistency more effectively than just allocating the memory region, as the majority of memory consumption is in buffer processing rather than executable code. Therefore, our priority has been to select the core. However, we can provide an option that allows both core selection and memory region allocation if a specific flag is set.

@Gabsuren and @euripedesrocha, here are my thoughts:

  1. I have implemented and am currently testing task creation with xTaskCreatePinnedToCore.
  2. I can add an option to enable the WebSocket Client to use xTaskCreateWithCaps.
  3. We can set a configuration flag to allow users to choose from three types of task creation:
    • xTaskCreatePinnedToCore
    • xTaskCreateWithCaps
    • A combination of both (with core selection and memory region allocation)

However, there is an issue with SDK versions prior to 5.1, which do not support xTaskCreatePinnedToCoreWithCaps. For these versions, we will need to use xTaskCreateStaticPinnedToCore and manually allocate the required memory size. Alternatively, we can use xTaskCreatePinnedToCoreWithCaps only if the SDK version is greater than 5.1. This approach ensures resource optimization and compatibility.

Any ideas or feedback on this?

filzek avatar Jul 24 '24 12:07 filzek

+1 Maybe allocate websocket task stack also from selected RAM by using xTaskCreateWithCaps?

if (!client->core) { // No core affinity if (!client->memory_type) { // Default memory if (xTaskCreate(esp_websocket_client_task, client->config->task_name ? client->config->task_name : "websocket_task", client->config->task_stack, client, client->config->task_prio, &client->task_handle) != pdTRUE) { ESP_LOGE(TAG, "Failed to create websocket task without core affinity using default memory"); return ESP_FAIL; } ESP_LOGD(TAG, "Websocket task created without core affinity using default memory"); } else { // Specific memory type without core affinity if (xTaskCreatePinnedToCoreWithCaps(esp_websocket_client_task, client->config->task_name ? client->config->task_name : "websocket_task", client->config->task_stack, client, client->config->task_prio, &client->task_handle, tskNO_AFFINITY, client->memory_type) != pdTRUE) { ESP_LOGE(TAG, "Failed to create websocket task without core affinity using specified memory type"); return ESP_FAIL; } ESP_LOGD(TAG, "Websocket task created without core affinity using specified memory type"); } } else { // With core affinity if (!client->memory_type) { // Default memory with core affinity if (xTaskCreatePinnedToCoreWithCaps(esp_websocket_client_task, client->config->task_name ? client->config->task_name : "websocket_task", client->config->task_stack, client, client->config->task_prio, &client->task_handle, client->core, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) != pdTRUE) { ESP_LOGE(TAG, "Failed to create websocket task with core affinity using default memory (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)"); return ESP_FAIL; } ESP_LOGD(TAG, "Websocket task created with core affinity using default memory (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)"); } else { // Specific memory type with core affinity if (xTaskCreatePinnedToCoreWithCaps(esp_websocket_client_task, client->config->task_name ? client->config->task_name : "websocket_task", client->config->task_stack, client, client->config->task_prio, &client->task_handle, client->core, client->memory_type) != pdTRUE) { ESP_LOGE(TAG, "Failed to create websocket task with core affinity using specified memory type"); return ESP_FAIL; } ESP_LOGD(TAG, "Websocket task created with core affinity using specified memory type (%d)", client->memory_type); } }

Now it is working correct using the memory area, but we need to evaluate to tell that some internal heap memory will be used as the task function couldnt be placed in the external memory.

filzek avatar Jul 31 '24 14:07 filzek

@filzek thank you for your contribution and diligent answer to our questions. I left a few comments that we have to address prior to merging. You will need also to clean up the commit history, could you please install the pre-commit as suggested in our contributing guide and have the commit messages in the correct format?

euripedesrocha avatar Aug 01 '24 07:08 euripedesrocha