8000
Skip to content

[core] Restore COMPONENT_STATE_LOOP_DONE check in calculate_looping_components - #9832

Merged
jesserockz merged 1 commit into
devfrom
loop_fix
Jul 23, 2025
Merged

[core] Restore COMPONENT_STATE_LOOP_DONE check in calculate_looping_components#9832
jesserockz merged 1 commit into
devfrom
loop_fix

Conversation

@bdraco
@bdraco bdraco commented Jul 23, 2025
Copy link
Copy Markdown
Member

What does this implement/fix?

This partially reverts PR #9820 to fix a performance regression where the logger component consumes excessive CPU cycles by running its loop continuously even when the buffer is empty.

The original PR removed the COMPONENT_STATE_LOOP_DONE check when building the looping components list, which broke components that call disable_loop() during initialization (before setup runs). The logger component specifically calls disable_loop() in init_log_buffer() to save CPU cycles when its buffer is empty, but this optimization was inadvertently broken.

This fix restores the state check while maintaining the early initialization of looping_components_ that was the main goal of PR #9820. While this reverts the 76-byte flash savings from the original PR, the performance impact of excessive CPU usage far outweighs the minimal flash benefit.

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)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

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

  • N/A

Test Environment

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

Example entry for config.yaml:

# No configuration changes needed
# This affects all ESP32 configurations with logger enabled
logger:
  level: DEBUG

Checklist:

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

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

Additional context

The runtime statistics before this fix show the logger consuming excessive resources:

[23:26:25][I][runtime_stats:060]:   logger: count=6920, avg=0.64ms, max=10ms, total=4413ms

This represents 6920 calls in 60 seconds (115 calls/second) totaling 4.4 seconds of CPU time out of 60 seconds.

The issue occurs because:

  1. Logger's init_log_buffer() calls disable_loop() to set state to COMPONENT_STATE_LOOP_DONE
  2. The simplified calculate_looping_components_() ignores this state and adds the logger to active components
  3. Logger's loop() runs continuously even with an empty buffer

This fix restores the original behavior where components in COMPONENT_STATE_LOOP_DONE are properly placed in the inactive section of the looping components list.

Trade-off: This restores the second loop that was removed for a 76-byte flash optimization, but the excessive CPU usage makes that optimization counterproductive.

Copilot AI review requested due to automatic review settings July 23, 2025 09:50
@bdraco
bdraco requested a review from a team as a code owner July 23, 2025 09:50
@esphome esphome Bot added core small-pr PR < 30 lines labels Jul 23, 2025
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 fixes a performance regression where the logger component consumes excessive CPU cycles by restoring the COMPONENT_STATE_LOOP_DONE check that was removed in PR #9820. The issue occurs when components call disable_loop() during initialization, but are still added to the active looping components list, causing unnecessary loop execution.

  • Restores state checking in calculate_looping_components_() to respect components in LOOP_DONE state
  • Adds separate handling for components that are already LOOP_DONE during initialization
  • Maintains the early initialization benefits from the original PR while fixing the CPU performance issue

Comment thread esphome/core/application.cpp
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.92%. Comparing base (bb6f8ae) to head (77c8363).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9832      +/-   ##
==========================================
- Coverage   59.96%   59.92%   -0.04%     
==========================================
  Files          51       51              
  Lines       10333    10333              
  Branches     1387     1387              
==========================================
- Hits         6196     6192       -4     
- Misses       3774     3777       +3     
- Partials      363      364       +1     

☔ 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.

@jesserockz
jesserockz merged commit 378b687 into dev Jul 23, 2025
30 checks passed
@jesserockz
jesserockz deleted the loop_fix branch July 23, 2025 11:31
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

core small-pr PR < 30 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0