From acc8b57709bb9b5dc1c3fd9704275e17923b79ab Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 20 Jul 2025 09:18:52 -1000 Subject: [PATCH] [api] Reduce memory usage by eliminating duplicate client info strings --- esphome/components/api/api_connection.cpp | 45 ++++++++++----------- esphome/components/api/api_connection.h | 27 ++++++++----- esphome/components/api/api_frame_helper.cpp | 15 +++---- esphome/components/api/api_frame_helper.h | 11 +++-- esphome/components/api/api_server.cpp | 6 +-- 5 files changed, 57 insertions(+), 47 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 2ac3303691..0a3923b679 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -105,13 +105,13 @@ void APIConnection::start() { APIError err = this->helper_->init(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Helper init failed %s errno=%d", this->get_client_combined_info().c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Helper init failed %s errno=%d", this->client_info_.get_combined_info().c_str(), + api_error_to_str(err), errno); return; } - this->client_info_ = helper_->getpeername(); - this->client_peername_ = this->client_info_; - this->helper_->set_log_info(this->client_info_); + this->client_info_.peername = helper_->getpeername(); + this->client_info_.name = this->client_info_.peername; + this->helper_->set_client_info(&this->client_info_); } APIConnection::~APIConnection() { @@ -138,7 +138,7 @@ void APIConnection::loop() { APIError err = this->helper_->loop(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Socket operation failed %s errno=%d", this->get_client_combined_info().c_str(), + ESP_LOGW(TAG, "%s: Socket operation failed %s errno=%d", this->client_info_.get_combined_info().c_str(), api_error_to_str(err), errno); return; } @@ -155,8 +155,8 @@ void APIConnection::loop() { break; } else if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Reading failed %s errno=%d", this->get_client_combined_info().c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Reading failed %s errno=%d", this->client_info_.get_combined_info().c_str(), + api_error_to_str(err), errno); return; } else { this->last_traffic_ = now; @@ -197,7 +197,7 @@ void APIConnection::loop() { // Disconnect if not responded within 2.5*keepalive if (now - this->last_traffic_ > KEEPALIVE_DISCONNECT_TIMEOUT) { on_fatal_error(); - ESP_LOGW(TAG, "%s is unresponsive; disconnecting", this->get_client_combined_info().c_str()); + ESP_LOGW(TAG, "%s is unresponsive; disconnecting", this->client_info_.get_combined_info().c_str()); } } else if (now - this->last_traffic_ > KEEPALIVE_TIMEOUT_MS && !this->flags_.remove) { // Only send ping if we're not disconnecting @@ -265,7 +265,7 @@ DisconnectResponse APIConnection::disconnect(const DisconnectRequest &msg) { // remote initiated disconnect_client // don't close yet, we still need to send the disconnect response // close will happen on next loop - ESP_LOGD(TAG, "%s disconnected", this->get_client_combined_info().c_str()); + ESP_LOGD(TAG, "%s disconnected", this->client_info_.get_combined_info().c_str()); this->flags_.next_close = true; DisconnectResponse resp; return resp; @@ -1409,9 +1409,9 @@ void APIConnection::complete_authentication_() { } this->flags_.connection_state = static_cast(ConnectionState::AUTHENTICATED); - ESP_LOGD(TAG, "%s connected", this->get_client_combined_info().c_str()); + ESP_LOGD(TAG, "%s connected", this->client_info_.get_combined_info().c_str()); #ifdef USE_API_CLIENT_CONNECTED_TRIGGER - this->parent_->get_client_connected_trigger()->trigger(this->client_info_, this->client_peername_); + this->parent_->get_client_connected_trigger()->trigger(this->client_info_.name, this->client_info_.peername); #endif #ifdef USE_HOMEASSISTANT_TIME if (homeassistant::global_homeassistant_time != nullptr) { @@ -1421,13 +1421,12 @@ void APIConnection::complete_authentication_() { } HelloResponse APIConnection::hello(const HelloRequest &msg) { - this->client_info_ = msg.client_info; - this->client_peername_ = this->helper_->getpeername(); - this->helper_->set_log_info(this->get_client_combined_info()); + this->client_info_.name = msg.client_info; + this->client_info_.peername = this->helper_->getpeername(); this->client_api_version_major_ = msg.api_version_major; this->client_api_version_minor_ = msg.api_version_minor; - ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.c_str(), - this->client_peername_.c_str(), this->client_api_version_major_, this->client_api_version_minor_); + ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.name.c_str(), + this->client_info_.peername.c_str(), this->client_api_version_major_, this->client_api_version_minor_); HelloResponse resp; resp.api_version_major = 1; @@ -1581,7 +1580,7 @@ bool APIConnection::try_to_clear_buffer(bool log_out_of_space) { APIError err = this->helper_->loop(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Socket operation failed %s errno=%d", this->get_client_combined_info().c_str(), + ESP_LOGW(TAG, "%s: Socket operation failed %s errno=%d", this->client_info_.get_combined_info().c_str(), api_error_to_str(err), errno); return false; } @@ -1602,7 +1601,7 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) { return false; if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Packet write failed %s errno=%d", this->get_client_combined_info().c_str(), + ESP_LOGW(TAG, "%s: Packet write failed %s errno=%d", this->client_info_.get_combined_info().c_str(), api_error_to_str(err), errno); return false; } @@ -1611,11 +1610,11 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) { } void APIConnection::on_unauthenticated_access() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s access without authentication", this->get_client_combined_info().c_str()); + ESP_LOGD(TAG, "%s access without authentication", this->client_info_.get_combined_info().c_str()); } void APIConnection::on_no_setup_connection() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s access without full connection", this->get_client_combined_info().c_str()); + ESP_LOGD(TAG, "%s access without full connection", this->client_info_.get_combined_info().c_str()); } void APIConnection::on_fatal_error() { this->helper_->close(); @@ -1787,8 +1786,8 @@ void APIConnection::process_batch_() { this->helper_->write_protobuf_packets(ProtoWriteBuffer{&this->parent_->get_shared_buffer_ref()}, packet_info); if (err != APIError::OK && err != APIError::WOULD_BLOCK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Batch write failed %s errno=%d", this->get_client_combined_info().c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Batch write failed %s errno=%d", this->client_info_.get_combined_info().c_str(), + api_error_to_str(err), errno); } #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 9ed18c24dc..b78c453d9a 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -16,6 +16,20 @@ namespace esphome { namespace api { +// Client information structure +struct ClientInfo { + std::string name; // Client name from Hello message + std::string peername; // IP:port from socket + + std::string get_combined_info() const { + if (name == peername) { + // Before Hello message, both are the same + return name; + } + return name + " (" + peername + ")"; + } +}; + // Keepalive timeout in milliseconds static constexpr uint32_t KEEPALIVE_TIMEOUT_MS = 60000; // Maximum number of entities to process in a single batch during initial state/info sending @@ -261,14 +275,6 @@ class APIConnection : public APIServerConnection { bool try_to_clear_buffer(bool log_out_of_space); bool send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) override; - std::string get_client_combined_info() const { - if (this->client_info_ == this->client_peername_) { - // Before Hello message, both are the same (just IP:port) - return this->client_info_; - } - return this->client_info_ + " (" + this->client_peername_ + ")"; - } - // Buffer allocator methods for batch processing ProtoWriteBuffer allocate_single_message_buffer(uint16_t size); ProtoWriteBuffer allocate_batch_message_buffer(uint16_t size); @@ -473,9 +479,8 @@ class APIConnection : public APIServerConnection { std::unique_ptr image_reader_; #endif - // Group 3: Strings (12 bytes each on 32-bit, 4-byte aligned) - std::string client_info_; - std::string client_peername_; + // Group 3: Client info struct (24 bytes on 32-bit: 2 strings × 12 bytes each) + ClientInfo client_info_; // Group 4: 4-byte types uint32_t last_traffic_; diff --git a/esphome/components/api/api_frame_helper.cpp b/esphome/components/api/api_frame_helper.cpp index afd64e8981..f91fa330d0 100644 --- a/esphome/components/api/api_frame_helper.cpp +++ b/esphome/components/api/api_frame_helper.cpp @@ -1,5 +1,6 @@ #include "api_frame_helper.h" #ifdef USE_API +#include "api_connection.h" // For ClientInfo struct #include "esphome/core/application.h" #include "esphome/core/hal.h" #include "esphome/core/helpers.h" @@ -13,6 +14,8 @@ namespace api { static const char *const TAG = "api.socket"; +#define HELPER_LOG(msg, ...) ESP_LOGVV(TAG, "%s: " msg, this->client_info_->get_combined_info().c_str(), ##__VA_ARGS__) + const char *api_error_to_str(APIError err) { // not using switch to ensure compiler doesn't try to build a big table out of it if (err == APIError::OK) { @@ -130,7 +133,7 @@ APIError APIFrameHelper::write_raw_(const struct iovec *iov, int iovcnt) { return APIError::OK; // Success, data buffered } // Socket error - ESP_LOGVV(TAG, "%s: Socket write failed with errno %d", this->info_.c_str(), errno); + HELPER_LOG("Socket write failed with errno %d", errno); this->state_ = State::FAILED; return APIError::SOCKET_WRITE_FAILED; // Socket write failed } else if (static_cast(sent) < total_write_len) { @@ -175,7 +178,7 @@ APIError APIFrameHelper::try_send_tx_buf_() { if (sent == -1) { if (errno != EWOULDBLOCK && errno != EAGAIN) { // Real socket error (not just would block) - ESP_LOGVV(TAG, "%s: Socket write failed with errno %d", this->info_.c_str(), errno); + HELPER_LOG("Socket write failed with errno %d", errno); this->state_ = State::FAILED; return APIError::SOCKET_WRITE_FAILED; // Socket write failed } @@ -203,13 +206,13 @@ APIError APIFrameHelper::try_send_tx_buf_() { APIError APIFrameHelper::init_common_() { if (state_ != State::INITIALIZE || this->socket_ == nullptr) { - ESP_LOGVV(TAG, "%s: Bad state for init %d", this->info_.c_str(), (int) state_); + HELPER_LOG("Bad state for init %d", (int) state_); return APIError::BAD_STATE; } int err = this->socket_->setblocking(false); if (err != 0) { state_ = State::FAILED; - ESP_LOGVV(TAG, "%s: Setting nonblocking failed with errno %d", this->info_.c_str(), errno); + HELPER_LOG("Setting nonblocking failed with errno %d", errno); return APIError::TCP_NONBLOCKING_FAILED; } @@ -217,14 +220,12 @@ APIError APIFrameHelper::init_common_() { err = this->socket_->setsockopt(IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(int)); if (err != 0) { state_ = State::FAILED; - ESP_LOGVV(TAG, "%s: Setting nodelay failed with errno %d", this->info_.c_str(), errno); + HELPER_LOG("Setting nodelay failed with errno %d", errno); return APIError::TCP_NODELAY_FAILED; } return APIError::OK; } -#define HELPER_LOG(msg, ...) ESP_LOGVV(TAG, "%s: " msg, this->info_.c_str(), ##__VA_ARGS__) - APIError APIFrameHelper::handle_socket_read_result_(ssize_t received) { if (received == -1) { if (errno == EWOULDBLOCK || errno == EAGAIN) { diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index 4bcc4acd61..3bc8700699 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -19,6 +19,9 @@ namespace esphome { namespace api { +// Forward declaration +struct ClientInfo; + class ProtoWriteBuffer; struct ReadPacketBuffer { @@ -94,8 +97,8 @@ class APIFrameHelper { } return APIError::OK; } - // Give this helper a name for logging - void set_log_info(std::string info) { info_ = std::move(info); } + // Set client info for logging + void set_client_info(const ClientInfo *client_info) { client_info_ = client_info; } virtual APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) = 0; // Write multiple protobuf packets in a single operation // packets contains (message_type, offset, length) for each message in the buffer @@ -161,10 +164,12 @@ class APIFrameHelper { // Containers (size varies, but typically 12+ bytes on 32-bit) std::deque tx_buf_; - std::string info_; std::vector reusable_iovs_; std::vector rx_buf_; + // Pointer to client info (4 bytes on 32-bit) + const ClientInfo *client_info_{nullptr}; + // Group smaller types together uint16_t rx_buf_len_ = 0; State state_{State::INITIALIZE}; diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 78c04f79c2..8c882b1856 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -166,7 +166,7 @@ void APIServer::loop() { // Network is down - disconnect all clients for (auto &client : this->clients_) { client->on_fatal_error(); - ESP_LOGW(TAG, "%s: Network down; disconnect", client->get_client_combined_info().c_str()); + ESP_LOGW(TAG, "%s: Network down; disconnect", client->client_info_.get_combined_info().c_str()); } // Continue to process and clean up the clients below } @@ -184,9 +184,9 @@ void APIServer::loop() { // Rare case: handle disconnection #ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER - this->client_disconnected_trigger_->trigger(client->client_info_, client->client_peername_); + this->client_disconnected_trigger_->trigger(client->client_info_.name, client->client_info_.peername); #endif - ESP_LOGV(TAG, "Remove connection %s", client->client_info_.c_str()); + ESP_LOGV(TAG, "Remove connection %s", client->client_info_.name.c_str()); // Swap with the last element and pop (avoids expensive vector shifts) if (client_index < this->clients_.size() - 1) {