diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d87945f8df..d5f9bdca13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -561,14 +561,26 @@ jobs: python script/ci_memory_impact_extract.py --output-env - name: Find and upload ELF file run: | - # Find the most recently created .elf file in .esphome/build - elf_file=$(find ~/.esphome/build -name "*.elf" -type f -printf '%T@ %p\n' 2>/dev/null | sort -rn | head -1 | cut -d' ' -f2-) + # Find the ELF file - try both common locations + elf_file="" + + # Try .esphome/build first (default location) + if [ -d ~/.esphome/build ]; then + elf_file=$(find ~/.esphome/build -name "firmware.elf" -o -name "*.elf" | head -1) + fi + + # Fallback to finding in .platformio if not found + if [ -z "$elf_file" ] && [ -d ~/.platformio ]; then + elf_file=$(find ~/.platformio -name "firmware.elf" | head -1) + fi + if [ -n "$elf_file" ] && [ -f "$elf_file" ]; then echo "Found ELF file: $elf_file" mkdir -p ./elf-artifacts cp "$elf_file" ./elf-artifacts/target.elf else - echo "Warning: No ELF file found" + echo "Warning: No ELF file found in ~/.esphome/build or ~/.platformio" + ls -la ~/.esphome/build/ || true fi - name: Upload ELF artifact uses: actions/upload-artifact@ea05be8e2b5c27c5689e977ed6f65db0a051b1e5 # v4.6.0 @@ -614,14 +626,26 @@ jobs: python script/ci_memory_impact_extract.py --output-env - name: Find and upload ELF file run: | - # Find the most recently created .elf file in .esphome/build - elf_file=$(find ~/.esphome/build -name "*.elf" -type f -printf '%T@ %p\n' 2>/dev/null | sort -rn | head -1 | cut -d' ' -f2-) + # Find the ELF file - try both common locations + elf_file="" + + # Try .esphome/build first (default location) + if [ -d ~/.esphome/build ]; then + elf_file=$(find ~/.esphome/build -name "firmware.elf" -o -name "*.elf" | head -1) + fi + + # Fallback to finding in .platformio if not found + if [ -z "$elf_file" ] && [ -d ~/.platformio ]; then + elf_file=$(find ~/.platformio -name "firmware.elf" | head -1) + fi + if [ -n "$elf_file" ] && [ -f "$elf_file" ]; then echo "Found ELF file: $elf_file" mkdir -p ./elf-artifacts cp "$elf_file" ./elf-artifacts/pr.elf else - echo "Warning: No ELF file found" + echo "Warning: No ELF file found in ~/.esphome/build or ~/.platformio" + ls -la ~/.esphome/build/ || true fi - name: Upload ELF artifact uses: actions/upload-artifact@ea05be8e2b5c27c5689e977ed6f65db0a051b1e5 # v4.6.0 @@ -651,6 +675,18 @@ jobs: with: python-version: ${{ env.DEFAULT_PYTHON }} cache-key: ${{ needs.common.outputs.cache-key }} + - name: Download target ELF artifact + uses: actions/download-artifact@1a18f44933c290e06e7167a92071e78bb20ab94a # v4.4.2 + with: + name: memory-impact-target-elf + path: ./elf-artifacts/target + continue-on-error: true + - name: Download PR ELF artifact + uses: actions/download-artifact@1a18f44933c290e06e7167a92071e78bb20ab94a # v4.4.2 + with: + name: memory-impact-pr-elf + path: ./elf-artifacts/pr + continue-on-error: true - name: Post or update PR comment env: GH_TOKEN: ${{ github.token }} @@ -662,6 +698,25 @@ jobs: PR_FLASH: ${{ needs.memory-impact-pr-branch.outputs.flash_usage }} run: | . venv/bin/activate + + # Check if ELF files exist + target_elf_arg="" + pr_elf_arg="" + + if [ -f ./elf-artifacts/target/target.elf ]; then + echo "Found target ELF file" + target_elf_arg="--target-elf ./elf-artifacts/target/target.elf" + else + echo "No target ELF file found" + fi + + if [ -f ./elf-artifacts/pr/pr.elf ]; then + echo "Found PR ELF file" + pr_elf_arg="--pr-elf ./elf-artifacts/pr/pr.elf" + else + echo "No PR ELF file found" + fi + python script/ci_memory_impact_comment.py \ --pr-number "${{ github.event.pull_request.number }}" \ --component "$COMPONENT" \ @@ -669,7 +724,9 @@ jobs: --target-ram "$TARGET_RAM" \ --target-flash "$TARGET_FLASH" \ --pr-ram "$PR_RAM" \ - --pr-flash "$PR_FLASH" + --pr-flash "$PR_FLASH" \ + $target_elf_arg \ + $pr_elf_arg ci-status: name: CI Status diff --git a/script/ci_memory_impact_comment.py b/script/ci_memory_impact_comment.py index f724a77c67..0b3bf87590 100755 --- a/script/ci_memory_impact_comment.py +++ b/script/ci_memory_impact_comment.py @@ -73,7 +73,7 @@ def format_change(before: int, after: int) -> str: def run_detailed_analysis( elf_path: str, objdump_path: str | None = None, readelf_path: str | None = None -) -> dict | None: +) -> tuple[dict | None, dict | None]: """Run detailed memory analysis on an ELF file. Args: @@ -82,16 +82,18 @@ def run_detailed_analysis( readelf_path: Optional path to readelf tool Returns: - Dictionary with component memory breakdown or None if analysis fails + Tuple of (component_breakdown, symbol_map) or (None, None) if analysis fails + component_breakdown: Dictionary with component memory breakdown + symbol_map: Dictionary mapping symbol names to their sizes """ try: analyzer = MemoryAnalyzer(elf_path, objdump_path, readelf_path) components = analyzer.analyze() # Convert ComponentMemory objects to dictionaries - result = {} + component_result = {} for name, mem in components.items(): - result[name] = { + component_result[name] = { "text": mem.text_size, "rodata": mem.rodata_size, "data": mem.data_size, @@ -100,10 +102,151 @@ def run_detailed_analysis( "ram_total": mem.ram_total, "symbol_count": mem.symbol_count, } - return result + + # Build symbol map from all sections + symbol_map = {} + for section in analyzer.sections.values(): + for symbol_name, size, _ in section.symbols: + if size > 0: # Only track non-zero sized symbols + # Demangle the symbol for better readability + demangled = analyzer._demangle_symbol(symbol_name) + symbol_map[demangled] = size + + return component_result, symbol_map except Exception as e: print(f"Warning: Failed to run detailed analysis: {e}", file=sys.stderr) - return None + import traceback + + traceback.print_exc(file=sys.stderr) + return None, None + + +def create_symbol_changes_table( + target_symbols: dict | None, pr_symbols: dict | None +) -> str: + """Create a markdown table showing symbols that changed size. + + Args: + target_symbols: Symbol name to size mapping for target branch + pr_symbols: Symbol name to size mapping for PR branch + + Returns: + Formatted markdown table + """ + if not target_symbols or not pr_symbols: + return "" + + # Find all symbols that exist in both branches or only in one + all_symbols = set(target_symbols.keys()) | set(pr_symbols.keys()) + + # Track changes + changed_symbols = [] + new_symbols = [] + removed_symbols = [] + + for symbol in all_symbols: + target_size = target_symbols.get(symbol, 0) + pr_size = pr_symbols.get(symbol, 0) + + if target_size == 0 and pr_size > 0: + # New symbol + new_symbols.append((symbol, pr_size)) + elif target_size > 0 and pr_size == 0: + # Removed symbol + removed_symbols.append((symbol, target_size)) + elif target_size != pr_size: + # Changed symbol + delta = pr_size - target_size + changed_symbols.append((symbol, target_size, pr_size, delta)) + + if not changed_symbols and not new_symbols and not removed_symbols: + return "" + + lines = [ + "", + "
", + "🔍 Symbol-Level Changes (click to expand)", + "", + ] + + # Show changed symbols (sorted by absolute delta) + if changed_symbols: + changed_symbols.sort(key=lambda x: abs(x[3]), reverse=True) + lines.extend( + [ + "### Changed Symbols", + "", + "| Symbol | Target Size | PR Size | Change |", + "|--------|-------------|---------|--------|", + ] + ) + + # Show top 30 changes + for symbol, target_size, pr_size, delta in changed_symbols[:30]: + target_str = format_bytes(target_size) + pr_str = format_bytes(pr_size) + change_str = format_change(target_size, pr_size) + # Truncate very long symbol names + display_symbol = symbol if len(symbol) <= 80 else symbol[:77] + "..." + lines.append( + f"| `{display_symbol}` | {target_str} | {pr_str} | {change_str} |" + ) + + if len(changed_symbols) > 30: + lines.append( + f"| ... | ... | ... | *({len(changed_symbols) - 30} more changed symbols not shown)* |" + ) + lines.append("") + + # Show new symbols + if new_symbols: + new_symbols.sort(key=lambda x: x[1], reverse=True) + lines.extend( + [ + "### New Symbols (top 15)", + "", + "| Symbol | Size |", + "|--------|------|", + ] + ) + + for symbol, size in new_symbols[:15]: + display_symbol = symbol if len(symbol) <= 80 else symbol[:77] + "..." + lines.append(f"| `{display_symbol}` | {format_bytes(size)} |") + + if len(new_symbols) > 15: + total_new_size = sum(s[1] for s in new_symbols) + lines.append( + f"| *{len(new_symbols) - 15} more new symbols...* | *Total: {format_bytes(total_new_size)}* |" + ) + lines.append("") + + # Show removed symbols + if removed_symbols: + removed_symbols.sort(key=lambda x: x[1], reverse=True) + lines.extend( + [ + "### Removed Symbols (top 15)", + "", + "| Symbol | Size |", + "|--------|------|", + ] + ) + + for symbol, size in removed_symbols[:15]: + display_symbol = symbol if len(symbol) <= 80 else symbol[:77] + "..." + lines.append(f"| `{display_symbol}` | {format_bytes(size)} |") + + if len(removed_symbols) > 15: + total_removed_size = sum(s[1] for s in removed_symbols) + lines.append( + f"| *{len(removed_symbols) - 15} more removed symbols...* | *Total: {format_bytes(total_removed_size)}* |" + ) + lines.append("") + + lines.extend(["
", ""]) + + return "\n".join(lines) def create_detailed_breakdown_table( @@ -148,7 +291,7 @@ def create_detailed_breakdown_table( lines = [ "", "
", - "📊 Detailed Memory Breakdown (click to expand)", + "📊 Component Memory Breakdown (click to expand)", "", "| Component | Target Flash | PR Flash | Change |", "|-----------|--------------|----------|--------|", @@ -205,19 +348,29 @@ def create_comment_body( # Run detailed analysis if ELF files are provided target_analysis = None pr_analysis = None - detailed_breakdown = "" + target_symbols = None + pr_symbols = None + component_breakdown = "" + symbol_changes = "" if target_elf and pr_elf: print( f"Running detailed analysis on {target_elf} and {pr_elf}", file=sys.stderr ) - target_analysis = run_detailed_analysis(target_elf, objdump_path, readelf_path) - pr_analysis = run_detailed_analysis(pr_elf, objdump_path, readelf_path) + target_analysis, target_symbols = run_detailed_analysis( + target_elf, objdump_path, readelf_path + ) + pr_analysis, pr_symbols = run_detailed_analysis( + pr_elf, objdump_path, readelf_path + ) if target_analysis and pr_analysis: - detailed_breakdown = create_detailed_breakdown_table( + component_breakdown = create_detailed_breakdown_table( target_analysis, pr_analysis ) + + if target_symbols and pr_symbols: + symbol_changes = create_symbol_changes_table(target_symbols, pr_symbols) else: print("No ELF files provided, skipping detailed analysis", file=sys.stderr) @@ -231,7 +384,7 @@ def create_comment_body( |--------|--------------|---------|--------| | **RAM** | {format_bytes(target_ram)} | {format_bytes(pr_ram)} | {ram_change} | | **Flash** | {format_bytes(target_flash)} | {format_bytes(pr_flash)} | {flash_change} | -{detailed_breakdown} +{component_breakdown}{symbol_changes} --- *This analysis runs automatically when a single component changes. Memory usage is measured from a representative test configuration.* """