8000
Skip to content

[text] Avoid string copies in callbacks by passing const ref - #12504

Merged
swoboda1337 merged 1 commit into
devfrom
text_avoid_copies_const_ref
Dec 17, 2025
Merged

[text] Avoid string copies in callbacks by passing const ref#12504
swoboda1337 merged 1 commit into
devfrom
text_avoid_copies_const_ref

Conversation

@bdraco
@bdraco bdraco commented Dec 15, 2025
Copy link
Copy Markdown
Member

What does this implement/fix?

Reduces heap churn in text entity by changing callback signatures from std::function<void(std::string)> to std::function<void(const std::string &)>.

Previously, every callback invocation caused a string copy because the callback signature passed by value. With this change, strings are passed by const reference, eliminating unnecessary heap allocations.

Before: Each publish_state() call copied the string once per registered callback.
After: Strings are passed by reference - no copies for callbacks.

No lifetime issues: The string is always valid during callback execution because:

  1. Callbacks are invoked synchronously (not deferred)
  2. The string is already copied into this->state before callbacks run
  3. The original caller's string remains alive until publish_state() returns

This is a minor API change. Lambdas (the common pattern) continue to work unchanged:

// These all still work:
text->add_on_state_callback([](const std::string &value) { ... });
text->add_on_state_callback([](std::string value) { ... });

The only breaking case is explicitly-typed std::function variables:

// This would need to be updated:
std::function<void(std::string)> cb = ...;  // Change to std::function<void(const std::string &)>
text->add_on_state_callback(std::move(cb));

This pattern is extremely rare in practice.

Follow-up to #12503 which applied the same optimization to text_sensor.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Developer breaking change (an API change that could break external components)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

N/A

Pull request in esphome-docs with documentation (if applicable):

N/A - internal API change only

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx
  • nRF52840

Example entry for config.yaml:

# No config changes - internal optimizati
8000
on
text:
  - platform: template
    name: "Test"
    optimistic: true

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder). Existing integration tests in tests/integration/ cover text callbacks.

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated in esphome-docs. (N/A - no user-facing changes)

@esphome esphome Bot added code-quality component: text developer-breaking-change This PR includes changes that break APIs that are potentially used by external components. small-pr PR < 30 lines labels Dec 15, 2025
@codecov-commenter
codecov-commenter commented Dec 15, 2025
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.66%. Comparing base (4509628) to head (e27c693).
⚠️ Report is 3 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev   #12504   +/-   ##
=======================================
  Coverage   72.66%   72.66%           
=======================================
  Files          53       53           
  Lines       11193    11193           
  Branches     1517     1517           
=======================================
  Hits         8133     8133           
  Misses       2667     2667           
  Partials      393      393           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#12504
    components: [text]
    refresh: 1h

(Added by the PR bot)

@github-actions
Copy link
Copy Markdown
Contributor

Memory Impact Analysis

Components: text
Platform: esp8266-ard

Metric Target Branch This PR Change
RAM 29,028 bytes 29,028 bytes ➡️ +0 bytes (0.00%)
Flash 274,695 bytes 274,663 bytes 📉 ✅ -32 bytes (-0.01%)
📊 Component Memory Breakdown
Component Target Flash PR Flash Change
cpp_runtime 884 bytes 835 bytes 📉 -49 bytes (-5.54%)
[esphome]text 302 bytes 315 bytes 📈 🚨 +13 bytes (+4.30%)
🔍 Symbol-Level Changes (click to expand)

Changed Symbols

Symbol Target Size PR Size Change
esphome::text::Text::publish_state(std::__cxx11::basic_string<char, std::char_traits, std::...esphome::text::Text::publish_state(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)
234 bytes 247 bytes 📈 +13 bytes (+5.56%)
std::__cxx11::basic_string<char, std::char_traits, std::allocator >::basic_string(std...std::__cxx11::basic_string<char, std::char_traits, std::allocator >::basic_string(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)
98 bytes 110 bytes 📈 +12 bytes (+12.24%)

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a representative test configuration.

@bdraco
bdraco marked this pull request as ready for review December 15, 2025 19:07
Copilot AI review requested due to automatic review settings December 15, 2025 19:07
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes memory usage in the text entity component by changing callback signatures to pass strings by const reference instead of by value, eliminating unnecessary heap allocations on each callback invocation.

Key changes:

  • Modified callback signature from std::function<void(std::string)> to std::function<void(const std::string &)>
  • Updated both the method signature and internal CallbackManager template parameter
  • This is a minor API breaking change, though lambda usage patterns (the common case) remain compatible

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
esphome/components/text/text.h Updated callback signature in public API method and internal callback manager type
esphome/components/text/text.cpp Updated callback method implementation signature to match header

@github-actions
Copy link
Copy Markdown
Contributor

👋 Hi there! This PR modifies 2 file(s) with codeowners.

@mauritskorse - As codeowner(s) of the affected files, your review would be appreciated! 🙏

Note: Automatic review request may have failed, but you're still welcome to review.

@bdraco
bdraco commented Dec 16, 2025
Copy link
Copy Markdown
Member Author

Thanks

@swoboda1337
swoboda1337 merged commit 42e061c into dev Dec 17, 2025
47 of 48 checks passed
@swoboda1337
swoboda1337 deleted the text_avoid_copies_const_ref branch December 17, 2025 17:00
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

code-quality component: text developer-breaking-change This PR includes changes that break APIs that are potentially used by external components. small-pr PR < 30 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0