From af9ecf3429b410046162ba951b765ab8c3e256a8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Aug 2025 02:38:27 -0500 Subject: [PATCH 1/4] [esp32_ble] Optimize BLE event memory usage by eliminating std::vector overhead (#10247) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/esp32_ble/ble_event.h | 57 +++++++++++++++++------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 884fc9ba65..2c0ab1d34e 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -3,8 +3,7 @@ #ifdef USE_ESP32 #include // for offsetof -#include - +#include // for memcpy #include #include #include @@ -64,7 +63,7 @@ static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == si // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). // This class stores each event with minimal memory usage. -// GAP events (99% of traffic) don't have the vector overhead. +// GAP events (99% of traffic) don't have the heap allocation overhead. // GATTC/GATTS events use heap allocation for their param and data. // // Event flow: @@ -145,16 +144,18 @@ class BLEEvent { } if (this->type_ == GATTC) { delete this->event_.gattc.gattc_param; - delete this->event_.gattc.data; + delete[] this->event_.gattc.data; this->event_.gattc.gattc_param = nullptr; this->event_.gattc.data = nullptr; + this->event_.gattc.data_len = 0; return; } if (this->type_ == GATTS) { delete this->event_.gatts.gatts_param; - delete this->event_.gatts.data; + delete[] this->event_.gatts.data; this->event_.gatts.gatts_param = nullptr; this->event_.gatts.data = nullptr; + this->event_.gatts.data_len = 0; } } @@ -209,17 +210,19 @@ class BLEEvent { esp_gattc_cb_event_t gattc_event; esp_gatt_if_t gattc_if; esp_ble_gattc_cb_param_t *gattc_param; // Heap-allocated - std::vector *data; // Heap-allocated - } gattc; // 16 bytes (pointers only) + uint8_t *data; // Heap-allocated raw buffer (manually managed) + uint16_t data_len; // Track size separately + } gattc; // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { esp_gatts_cb_event_t gatts_event; esp_gatt_if_t gatts_if; esp_ble_gatts_cb_param_t *gatts_param; // Heap-allocated - std::vector *data; // Heap-allocated - } gatts; // 16 bytes (pointers only) - } event_; // 80 bytes + uint8_t *data; // Heap-allocated raw buffer (manually managed) + uint16_t data_len; // Track size separately + } gatts; + } event_; // 80 bytes ble_event_t type_; @@ -319,6 +322,7 @@ class BLEEvent { if (p == nullptr) { this->event_.gattc.gattc_param = nullptr; this->event_.gattc.data = nullptr; + this->event_.gattc.data_len = 0; return; // Invalid event, but we can't log in header file } @@ -336,16 +340,29 @@ class BLEEvent { // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTC_NOTIFY_EVT: - this->event_.gattc.data = new std::vector(p->notify.value, p->notify.value + p->notify.value_len); - this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data->data(); + this->event_.gattc.data_len = p->notify.value_len; + if (p->notify.value_len > 0) { + this->event_.gattc.data = new uint8_t[p->notify.value_len]; + memcpy(this->event_.gattc.data, p->notify.value, p->notify.value_len); + } else { + this->event_.gattc.data = nullptr; + } + this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data; break; case ESP_GATTC_READ_CHAR_EVT: case ESP_GATTC_READ_DESCR_EVT: - this->event_.gattc.data = new std::vector(p->read.value, p->read.value + p->read.value_len); - this->event_.gattc.gattc_param->read.value = this->event_.gattc.data->data(); + this->event_.gattc.data_len = p->read.value_len; + if (p->read.value_len > 0) { + this->event_.gattc.data = new uint8_t[p->read.value_len]; + memcpy(this->event_.gattc.data, p->read.value, p->read.value_len); + } else { + this->event_.gattc.data = nullptr; + } + this->event_.gattc.gattc_param->read.value = this->event_.gattc.data; break; default: this->event_.gattc.data = nullptr; + this->event_.gattc.data_len = 0; break; } } @@ -358,6 +375,7 @@ class BLEEvent { if (p == nullptr) { this->event_.gatts.gatts_param = nullptr; this->event_.gatts.data = nullptr; + this->event_.gatts.data_len = 0; return; // Invalid event, but we can't log in header file } @@ -375,11 +393,18 @@ class BLEEvent { // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTS_WRITE_EVT: - this->event_.gatts.data = new std::vector(p->write.value, p->write.value + p->write.len); - this->event_.gatts.gatts_param->write.value = this->event_.gatts.data->data(); + this->event_.gatts.data_len = p->write.len; + if (p->write.len > 0) { + this->event_.gatts.data = new uint8_t[p->write.len]; + memcpy(this->event_.gatts.data, p->write.value, p->write.len); + } else { + this->event_.gatts.data = nullptr; + } + this->event_.gatts.gatts_param->write.value = this->event_.gatts.data; break; default: this->event_.gatts.data = nullptr; + this->event_.gatts.data_len = 0; break; } } From 7005da42bb4933768d8cf8485baca81a637c38a3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Aug 2025 08:52:21 -0500 Subject: [PATCH 2/4] preen --- esphome/components/esp32_ble/ble_event.h | 132 ++++++++++++----------- 1 file changed, 68 insertions(+), 64 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index bdac1d6458..299fd7705f 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -61,9 +61,19 @@ static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.rssi) == sizeof(es static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == sizeof(esp_bt_status_t) + sizeof(int8_t), "remote_addr must follow rssi in read_rssi_cmpl"); -// Maximum size for inline storage of GATTC/GATTS data -// This value is chosen to fit within the 80-byte union while maximizing inline storage -static constexpr size_t INLINE_DATA_SIZE = 68; +// Param struct sizes on ESP32 +static constexpr size_t GATTC_PARAM_SIZE = 28; +static constexpr size_t GATTS_PARAM_SIZE = 32; + +// Maximum size for inline storage of data +// GATTC: 80 - 28 (param) - 8 (other fields) = 44 bytes for data +// GATTS: 80 - 32 (param) - 8 (other fields) = 40 bytes for data +static constexpr size_t GATTC_INLINE_DATA_SIZE = 44; +static constexpr size_t GATTS_INLINE_DATA_SIZE = 40; + +// Verify param struct sizes +static_assert(sizeof(esp_ble_gattc_cb_param_t) == GATTC_PARAM_SIZE, "GATTC param size unexpected"); +static_assert(sizeof(esp_ble_gatts_cb_param_t) == GATTS_PARAM_SIZE, "GATTS param size unexpected"); // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). // This class stores each event with minimal memory usage. @@ -115,21 +125,21 @@ class BLEEvent { this->init_gap_data_(e, p); } - // Constructor for GATTC events - uses heap allocation - // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. - // The param pointer from ESP-IDF is only valid during the callback execution. - // Since BLE events are processed asynchronously in the main loop, we must create - // our own copy to ensure the data remains valid until the event is processed. + // Constructor for GATTC events - param stored inline, data may use heap + // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF + // is only valid during the callback execution. Since BLE events are processed + // asynchronously in the main loop, we store our own copy inline to ensure + // the data remains valid until the event is processed. BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { this->type_ = GATTC; this->init_gattc_data_(e, i, p); } - // Constructor for GATTS events - uses heap allocation - // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. - // The param pointer from ESP-IDF is only valid during the callback execution. - // Since BLE events are processed asynchronously in the main loop, we must create - // our own copy to ensure the data remains valid until the event is processed. + // Constructor for GATTS events - param stored inline, data may use heap + // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF + // is only valid during the callback execution. Since BLE events are processed + // asynchronously in the main loop, we store our own copy inline to ensure + // the data remains valid until the event is processed. BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { this->type_ = GATTS; this->init_gatts_data_(e, i, p); @@ -148,24 +158,20 @@ class BLEEvent { // GAP events don't have heap allocations break; case GATTC: - delete this->event_.gattc.gattc_param; - // Only delete heap data if it was heap-allocated + // Param is now stored inline, only delete heap data if it was heap-allocated if (!this->event_.gattc.is_inline && this->event_.gattc.data.heap_data != nullptr) { delete[] this->event_.gattc.data.heap_data; } // Clear critical fields to prevent issues if type changes - this->event_.gattc.gattc_param = nullptr; this->event_.gattc.is_inline = false; this->event_.gattc.data.heap_data = nullptr; break; case GATTS: - delete this->event_.gatts.gatts_param; - // Only delete heap data if it was heap-allocated + // Param is now stored inline, only delete heap data if it was heap-allocated if (!this->event_.gatts.is_inline && this->event_.gatts.data.heap_data != nullptr) { delete[] this->event_.gatts.data.heap_data; } // Clear critical fields to prevent issues if type changes - this->event_.gatts.gatts_param = nullptr; this->event_.gatts.is_inline = false; this->event_.gatts.data.heap_data = nullptr; break; @@ -220,30 +226,30 @@ class BLEEvent { // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { - esp_ble_gattc_cb_param_t *gattc_param; // Heap-allocated (4 bytes) - esp_gattc_cb_event_t gattc_event; // 4 bytes + esp_ble_gattc_cb_param_t gattc_param; // Stored inline (28 bytes) + esp_gattc_cb_event_t gattc_event; // 4 bytes union { - uint8_t *heap_data; // 4 bytes when heap-allocated - uint8_t inline_data[INLINE_DATA_SIZE]; // INLINE_DATA_SIZE bytes when stored inline - } data; // INLINE_DATA_SIZE bytes total - uint16_t data_len; // 2 bytes - esp_gatt_if_t gattc_if; // 1 byte - bool is_inline; // 1 byte - true when data is stored inline - } gattc; // Total: 80 bytes + uint8_t *heap_data; // 4 bytes when heap-allocated + uint8_t inline_data[GATTC_INLINE_DATA_SIZE]; // 44 bytes when stored inline + } data; // 44 bytes total + uint16_t data_len; // 2 bytes + esp_gatt_if_t gattc_if; // 1 byte + bool is_inline; // 1 byte - true when data is stored inline + } gattc; // Total: 80 bytes // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { - esp_ble_gatts_cb_param_t *gatts_param; // Heap-allocated (4 bytes) - esp_gatts_cb_event_t gatts_event; // 4 bytes + esp_ble_gatts_cb_param_t gatts_param; // Stored inline (32 bytes) + esp_gatts_cb_event_t gatts_event; // 4 bytes union { - uint8_t *heap_data; // 4 bytes when heap-allocated - uint8_t inline_data[INLINE_DATA_SIZE]; // INLINE_DATA_SIZE bytes when stored inline - } data; // INLINE_DATA_SIZE bytes total - uint16_t data_len; // 2 bytes - esp_gatt_if_t gatts_if; // 1 byte - bool is_inline; // 1 byte - true when data is stored inline - } gatts; // Total: 80 bytes - } event_; // 80 bytes + uint8_t *heap_data; // 4 bytes when heap-allocated + uint8_t inline_data[GATTS_INLINE_DATA_SIZE]; // 40 bytes when stored inline + } data; // 40 bytes total + uint16_t data_len; // 2 bytes + esp_gatt_if_t gatts_if; // 1 byte + bool is_inline; // 1 byte - true when data is stored inline + } gatts; // Total: 80 bytes + } event_; // 80 bytes ble_event_t type_; @@ -258,12 +264,12 @@ class BLEEvent { private: // Helper to copy data with inline storage optimization - template + template void copy_data_with_inline_storage_(EventStruct &event, const uint8_t *src_data, uint16_t len, uint8_t **param_value_ptr) { event.data_len = len; if (len > 0) { - if (len <= INLINE_DATA_SIZE) { + if (len <= InlineSize) { event.is_inline = true; memcpy(event.data.inline_data, src_data, len); *param_value_ptr = event.data.inline_data; @@ -364,34 +370,33 @@ class BLEEvent { this->event_.gattc.gattc_if = i; if (p == nullptr) { - this->event_.gattc.gattc_param = nullptr; + // Zero out the param struct when null + memset(&this->event_.gattc.gattc_param, 0, sizeof(this->event_.gattc.gattc_param)); this->event_.gattc.is_inline = false; this->event_.gattc.data.heap_data = nullptr; this->event_.gattc.data_len = 0; return; // Invalid event, but we can't log in header file } - // Heap-allocate param - // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) - // while GAP events (99%) are stored inline to minimize memory usage - // IMPORTANT: This heap allocation provides clear ownership semantics: - // - The BLEEvent owns the allocated memory for its lifetime - // - The data remains valid from the BLE callback context until processed in the main loop - // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory - this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); + // Copy param struct inline (no heap allocation!) + // GATTC/GATTS events are rare (<1% of events) but we can still store them inline + // along with small data payloads, eliminating all heap allocations for typical BLE operations + // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer + // is only valid during the callback and will be reused/freed after we return + this->event_.gattc.gattc_param = *p; // Copy data for events that need it // The param struct contains pointers (e.g., notify.value) that point to temporary buffers. // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTC_NOTIFY_EVT: - copy_data_with_inline_storage_(this->event_.gattc, p->notify.value, p->notify.value_len, - &this->event_.gattc.gattc_param->notify.value); + copy_data_with_inline_storage_event_.gattc), GATTC_INLINE_DATA_SIZE>( + this->event_.gattc, p->notify.value, p->notify.value_len, &this->event_.gattc.gattc_param.notify.value); break; case ESP_GATTC_READ_CHAR_EVT: case ESP_GATTC_READ_DESCR_EVT: - copy_data_with_inline_storage_(this->event_.gattc, p->read.value, p->read.value_len, - &this->event_.gattc.gattc_param->read.value); + copy_data_with_inline_storage_event_.gattc), GATTC_INLINE_DATA_SIZE>( + this->event_.gattc, p->read.value, p->read.value_len, &this->event_.gattc.gattc_param.read.value); break; default: this->event_.gattc.is_inline = false; @@ -407,29 +412,28 @@ class BLEEvent { this->event_.gatts.gatts_if = i; if (p == nullptr) { - this->event_.gatts.gatts_param = nullptr; + // Zero out the param struct when null + memset(&this->event_.gatts.gatts_param, 0, sizeof(this->event_.gatts.gatts_param)); this->event_.gatts.is_inline = false; this->event_.gatts.data.heap_data = nullptr; this->event_.gatts.data_len = 0; return; // Invalid event, but we can't log in header file } - // Heap-allocate param - // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) - // while GAP events (99%) are stored inline to minimize memory usage - // IMPORTANT: This heap allocation provides clear ownership semantics: - // - The BLEEvent owns the allocated memory for its lifetime - // - The data remains valid from the BLE callback context until processed in the main loop - // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory - this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); + // Copy param struct inline (no heap allocation!) + // GATTC/GATTS events are rare (<1% of events) but we can still store them inline + // along with small data payloads, eliminating all heap allocations for typical BLE operations + // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer + // is only valid during the callback and will be reused/freed after we return + this->event_.gatts.gatts_param = *p; // Copy data for events that need it // The param struct contains pointers (e.g., write.value) that point to temporary buffers. // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTS_WRITE_EVT: - copy_data_with_inline_storage_(this->event_.gatts, p->write.value, p->write.len, - &this->event_.gatts.gatts_param->write.value); + copy_data_with_inline_storage_event_.gatts), GATTS_INLINE_DATA_SIZE>( + this->event_.gatts, p->write.value, p->write.len, &this->event_.gatts.gatts_param.write.value); break; default: this->event_.gatts.is_inline = false; From 3aae84fadef602789d89e1537bab0460906e4cda Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Aug 2025 08:54:17 -0500 Subject: [PATCH 3/4] preen --- esphome/components/esp32_ble/ble.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index d1ee7af4ea..a5ce83c2d8 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -306,7 +306,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTS: { esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; - esp_ble_gatts_cb_param_t *param = ble_event->event_.gatts.gatts_param; + esp_ble_gatts_cb_param_t *param = &ble_event->event_.gatts.gatts_param; // Take address of inline struct ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); for (auto *gatts_handler : this->gatts_event_handlers_) { gatts_handler->gatts_event_handler(event, gatts_if, param); @@ -316,7 +316,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTC: { esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; - esp_ble_gattc_cb_param_t *param = ble_event->event_.gattc.gattc_param; + esp_ble_gattc_cb_param_t *param = &ble_event->event_.gattc.gattc_param; // Take address of inline struct ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); for (auto *gattc_handler : this->gattc_event_handlers_) { gattc_handler->gattc_event_handler(event, gattc_if, param); From d78d2c87107767323745758b82bf99b4a3069d0d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Aug 2025 09:53:07 -0500 Subject: [PATCH 4/4] Apply suggestions from code review --- esphome/components/esp32_ble/ble.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index a5ce83c2d8..e22d43c0cc 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -306,7 +306,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTS: { esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; - esp_ble_gatts_cb_param_t *param = &ble_event->event_.gatts.gatts_param; // Take address of inline struct + esp_ble_gatts_cb_param_t *param = &ble_event->event_.gatts.gatts_param; ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); for (auto *gatts_handler : this->gatts_event_handlers_) { gatts_handler->gatts_event_handler(event, gatts_if, param); @@ -316,7 +316,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTC: { esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; - esp_ble_gattc_cb_param_t *param = &ble_event->event_.gattc.gattc_param; // Take address of inline struct + esp_ble_gattc_cb_param_t *param = &ble_event->event_.gattc.gattc_param; ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); for (auto *gattc_handler : this->gattc_event_handlers_) { gattc_handler->gattc_event_handler(event, gattc_if, param);