Skip to content

Fix performance penalty when sending tars to daemon by optimizing layer size calculation#292

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/fix-276
Draft

Fix performance penalty when sending tars to daemon by optimizing layer size calculation#292
Copilot wants to merge 3 commits intomainfrom
copilot/fix-276

Conversation

Copy link

Copilot AI commented Aug 7, 2025

This PR addresses the performance penalty incurred by sending legacy formatted tars to the Docker daemon, as identified in the getLayerSize() function comments.

Problem

The current implementation has a significant performance bottleneck when creating tars for the Docker daemon:

  1. For each layer, addLayerToTar() calls layer.Uncompressed() to get the layer data
  2. It then calls getLayerSize() to determine the uncompressed size for the tar header
  3. When the layer size is unknown, getLayerSize() calls layer.Uncompressed() again and reads through the entire layer with io.Copy(io.Discard, layerReader) just to calculate the size
  4. This results in decompressing each layer twice and an expensive discard operation

Solution

Added an optimization that detects when all layers have known uncompressed sizes (typically after docker image save operations) and uses a more efficient code path:

  • canOptimizeLayerSizes(): Checks if all image layers have cached uncompressed sizes from previous docker save operations
  • addLayerToTarOptimized(): Optimized version that uses cached sizes directly, avoiding the expensive size calculation
  • Modified addImageToTar(): Routes to the optimized path when beneficial

Performance Improvement

Before:

// For each layer:
layerReader := layer.Uncompressed()          // First decompression
size := getLayerSize(layer)                  // Calls layer.Uncompressed() AGAIN
    io.Copy(io.Discard, layerReader)         // Expensive discard operation

After (when optimization applies):

// For each layer:
size := knownLayer.uncompressedSize          // Use cached size
layerReader := layer.Uncompressed()          // Single decompression

This eliminates redundant layer decompression and the expensive discard operation when working with images that have all layers downloaded from the daemon.

The optimization is automatically applied when beneficial and falls back to the original implementation when layer sizes are unknown, ensuring backward compatibility.

Fixes #276.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • docker-images-prod.6aa30f8b08e16409b46e0173d6de2f56.r2.cloudflarestorage.com
    • Triggering command: /tmp/go-build2850353549/b355/remote.test -test.testlogfile=/tmp/go-build2850353549/b355/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true (dns block)
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)
  • eastus.data.mcr.microsoft.com
    • Triggering command: /tmp/go-build2850353549/b355/remote.test -test.testlogfile=/tmp/go-build2850353549/b355/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true (dns block)
  • myother-insecure-registry.com
    • Triggering command: /tmp/go-build2850353549/b355/remote.test -test.testlogfile=/tmp/go-build2850353549/b355/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Author

Copilot AI commented Aug 7, 2025

@jabrown85 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI and others added 2 commits August 7, 2025 14:32
…re known

Co-authored-by: jabrown85 <790999+jabrown85@users.noreply.github.com>
Co-authored-by: jabrown85 <790999+jabrown85@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix performance penalty incurred by sending legacy formatted tars to the daemon Fix performance penalty when sending tars to daemon by optimizing layer size calculation Aug 7, 2025
Copilot AI requested a review from jabrown85 August 7, 2025 14:38
@jjbustamante
Copy link
Member

@jabrown85 Did you manage to test this fix with Docker + Containerd storage performance issue?

@jabrown85
Copy link
Contributor

Sadly, no @jjbustamante. I think we would need to have access to the storage layer APIs to really improve the perf

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.

Fix performance penalty incurred by sending legacy formatted tars to the daemon

3 participants

Comments