Skip to content

docs(perf): #114 warm-deploy loop first-pass results#230

Merged
zackees merged 1 commit intomainfrom
feat/114-warm-deploy-loop-first-pass
May 10, 2026
Merged

docs(perf): #114 warm-deploy loop first-pass results#230
zackees merged 1 commit intomainfrom
feat/114-warm-deploy-loop-first-pass

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented May 10, 2026

Summary

Closes the acceptance loop on #114. First-pass measurement of the warm
rebuild + deploy + monitor reconnect path against real ESP32-S3 hardware
(ESP32-S3-DevKitC-1, native USB-CDC on COM13).

5/5 consecutive iterations of the full loop land at 3.585–3.697 s — all
inside the 4 000 ms budget
, lowest margin 303 ms. Acceptance criterion
(three consecutive in-budget iterations) is met with a margin.

iter full loop wall-clock margin vs 4 s
1 3.587 s +413 ms
2 3.590 s +410 ms
3 3.697 s +303 ms
4 3.585 s +415 ms
5 3.605 s +395 ms

Deploy-only (no monitor) is ~2.6 s steady-state.

No follow-up perf issues are filed: no phase consistently breaches its
sub-budget. The ~1 s extra between deploy-only and full loop is the test
sketch's delay(500)×2 loop period (next Serial.println cadence), not
deploy/reconnect infrastructure.

This PR adds docs/PERF_WARM_DEPLOY.md (mirrors the PERF_WARM_BUILD.md
shape used for #91) and adds the corresponding row to docs/INDEX.md.

Test plan

  • Cold deploy ESP32-S3 to a known image (3m 16s; full flash baseline)
  • 5 deploy-only warm iterations on COM13 — all verify skipped, device already matched
  • 5 full-loop iterations (--monitor --halt-on-success "Hello from ESP32-S3" --timeout 5) — all green inside 4 000 ms
  • Stop-hook lint + tests on docs-only change (no code edits)

Refs #114. The fbuild-side perf work that got us here landed previously
in #116 (trust-skip), #118 (ImageHashMemo + device refresh debounce),
and #120 (DaemonWatchSetCache).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added comprehensive performance benchmark report for warm rebuild, deploy, and monitor cycles on ESP32-S3 hardware, including test methodology and measured timing results (3.6-3.7 seconds per cycle).
    • Updated FAQ index to reference the new performance documentation.

Review Change Stack

5/5 iterations of `deploy + monitor reattach + first-byte-from-device`
land at 3.585-3.697 s on ESP32-S3-DevKitC-1, all inside the 4 000 ms
budget defined in #114. Closes the acceptance loop on #114; no
follow-up perf issues filed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

📝 Walkthrough

Walkthrough

A new performance report PERF_WARM_DEPLOY.md documents warm-deploy loop benchmark results on ESP32-S3 hardware, confirming the 4-second end-to-end budget for issue #114 with measured timing across five iterations (3.585–3.697s). An index entry in docs/INDEX.md links to this report.

Changes

Warm-Deploy Performance Acceptance

Layer / File(s) Summary
Benchmark Report
docs/PERF_WARM_DEPLOY.md
New performance document with title, TL;DR outcomes, testing methodology (host OS, device setup, cold-deploy precondition), deploy-only warm-path timings (~2.57–2.64s), and full warm-loop results across five consecutive runs (3.585–3.697s, all under 4-second budget). Records prior optimizations that enabled the results and confirms acceptance criteria are satisfied with #114 resolved and LOOP.md tracking retired.
Documentation Index
docs/INDEX.md
New FAQ table row added linking "warm rebuild + deploy + monitor under 4 s? (#114)" to PERF_WARM_DEPLOY.md.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Possibly related PRs

  • FastLED/fbuild#82: Both PRs modify docs/INDEX.md to update the documentation index; this PR adds a new FAQ row for issue #114's performance results.

Poem

🐰 A warm deploy, so swift and fleet,
Four seconds marked—the budget's beat!
Five runs confirmed, the margins slack,
The docs record what builds bring back.
No LOOP ahead, the loop is done!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: documenting first-pass warm-deploy loop benchmark results for issue #114.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/114-warm-deploy-loop-first-pass

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/PERF_WARM_DEPLOY.md`:
- Around line 72-73: The line begins with "#114." which Markdown parses as an
ATX heading; change that occurrence to inline text such as "issue `#114`." (or
wrap it in backticks `#114`) so it no longer starts the line and markdownlint
MD018 is not triggered—update the exact token "#114." in the affected paragraph
to "issue `#114`." to fix the linting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 28c37499-60cb-4c13-9469-32ec442d2982

📥 Commits

Reviewing files that changed from the base of the PR and between 342b6d2 and 774b9c7.

📒 Files selected for processing (2)
  • docs/INDEX.md
  • docs/PERF_WARM_DEPLOY.md

Comment thread docs/PERF_WARM_DEPLOY.md
Comment on lines +72 to +73
breaching its individual budget; no follow-up perf issues are filed against
#114.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Line 73: avoid accidental ATX heading syntax

#114. at start-of-line is parsed as a heading marker and triggers MD018. Keep it inline text (for example: issue #114.) so markdownlint stays clean.

Suggested fix
-The full loop runs cleanly inside the 4 s envelope without any phase
-breaching its individual budget; no follow-up perf issues are filed against
-#114.
+The full loop runs cleanly inside the 4 s envelope without any phase
+breaching its individual budget; no follow-up perf issues are filed against
+issue `#114`.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 73-73: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/PERF_WARM_DEPLOY.md` around lines 72 - 73, The line begins with "#114."
which Markdown parses as an ATX heading; change that occurrence to inline text
such as "issue `#114`." (or wrap it in backticks `#114`) so it no longer starts
the line and markdownlint MD018 is not triggered—update the exact token "#114."
in the affected paragraph to "issue `#114`." to fix the linting.

@zackees zackees merged commit bfd062f into main May 10, 2026
81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant