Skip to content

Comments

fix: Issue 681 - Image details render - Performance improvement (string += --> strings.Builder)#682

Open
huntharo wants to merge 2 commits intowagoodman:mainfrom
huntharo:issue-681/image-details-performance
Open

fix: Issue 681 - Image details render - Performance improvement (string += --> strings.Builder)#682
huntharo wants to merge 2 commits intowagoodman:mainfrom
huntharo:issue-681/image-details-performance

Conversation

@huntharo
Copy link

Summary

This PR isolates the image-details performance regression work into two focused commits:

  1. test: add image details perf regression coverage
  • Extract image details body generation into renderImageDetailsBody(...) in a dedicated file.
  • Add regression tests for:
    • O(n^2)-style string concatenation anti-pattern detection.
    • Output/content baseline validation.
    • Integration-style scaling behavior check.
  • This commit is intentionally red: regression tests fail before the fix.
  1. fix: avoid quadratic string concatenation in image details
  • Replace repeated inefficiencyReport += ... concatenation with strings.Builder + fmt.Fprintf.
  • Keep rendered content behavior unchanged while removing the quadratic concatenation pattern.

Why

On large images with many inefficiency rows (e.g. overlapping layers with massive path duplication), repeated string concatenation in the image details render path can produce severe CPU spikes and long (5-15 seconds) per-keypress latency when navigating across any layers (not just the problem layers).

Scope

  • cmd/dive/cli/internal/ui/v1/view/image_details.go
  • cmd/dive/cli/internal/ui/v1/view/image_details_content.go (new)
  • cmd/dive/cli/internal/ui/v1/view/image_details_regression_test.go (new)

No unrelated branch commits are included.

Validation

Commit 1 state (expected failing regression tests)

  • go test ./cmd/dive/cli/internal/ui/v1/view
    • Fails on regression tests (expected pre-fix).
  • go test ./cmd/dive/cli/internal/ui/v1/viewmodel ./cmd/dive/cli/internal/ui/v1/app ./dive/filetree
    • Passes.

Commit 2 state (fixed)

  • go test ./cmd/dive/cli/internal/ui/v1/view ./cmd/dive/cli/internal/ui/v1/viewmodel ./cmd/dive/cli/internal/ui/v1/app ./dive/filetree
    • Passes.

Revert check (proof tests catch regression)

In an isolated temp clone, revert commit 2 and run:

  • go test ./cmd/dive/cli/internal/ui/v1/view ./cmd/dive/cli/internal/ui/v1/viewmodel ./cmd/dive/cli/internal/ui/v1/app ./dive/filetree
  • Result: regression tests fail again (expected), confirming detection works.

Fixes #681

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.

Layers that change same files make dive unusably slow

1 participant