solver: run image and cache exports in parallel

[amrmahdi]

Image export and cache export currently run sequentially. This adds unnecessary latency when both are configured, especially when exporting to different destinations.

Tested with vLLM CI builds. A full build-from-cache takes about 7 minutes, with export dominating: image export runs for ~2min, then cache export for another ~2min. Running them in parallel cuts export time in half, saving ~2 minutes per build (~30% of total time).

The one exception is inline cache: it works by running the cache export first to generate metadata, which is then embedded into the image config before export. This means image export cannot start until inline cache completes, so we preserve sequential execution in that case.

[tonistiigi]

I forgot one other case that affects this. In ExportTo based on Mode, when cache export checks if result is to be exported, it checks if it already exists or not. Eg. if image exporter(or previous cache source) already generated the layer then it is exported in CacheExportModeMin but otherwise only metadata is exported and no new layer tarball is created by the exporter. I wonder that maybe we need to instead break the export phase into two passes so that the push can run in parallel, but creating the objects is still sequential.

[tonistiigi]

Windows CI doesn't seem to like this as well @rzlink

failed to commit ngtnv4s8gc8tp0beynhncp87s to oexb7oycg3u2l5y777xguo7u6 during finalize: failed to reimport snapshot: hcsshim::ActivateLayer failed in Win32: The process cannot access the file because it is being used by another process. (0x20)

[amrmahdi]

I forgot one other case that affects this. In ExportTo based on Mode, when cache export checks if result is to be exported, it checks if it already exists or not. Eg. if image exporter(or previous cache source) already generated the layer then it is exported in CacheExportModeMin but otherwise only metadata is exported and no new layer tarball is created by the exporter. I wonder that maybe we need to instead break the export phase into two passes so that the push can run in parallel, but creating the objects is still sequential.

@tonistiigi would a simplification be to treat CacheExportModeMin as sequential like the inline cache?

[amrmahdi]

I wonder that maybe we need to instead break the export phase into two passes so that the push can run in parallel, but creating the objects is still sequential.

@tonistiigi If we go with the 2-phase approach, we will need to change the interfaces to add the 2 export phases (maybe Prepare and Finalize), and we could keep Export as is for backward compatibility (internally it just calls Prepare + Finalize). If I understand correctly, Finalize will mostly be a no-op for most exporters. Does this align with your thoughts?

[tonistiigi]

would a simplification be to treat CacheExportModeMin as sequential like the inline cache?

I think it could still be different even with max. The blobs creation depends on how the compression algorithm and level was configured for exporter if the blobs are shared.

and we could keep Export as is for backward compatibility

In buildkit repo we don't care about backward compatibility of Go API. Only on-wire APIs. Either splitting in two or maybe Export can optionally return a "finalize" callback that needs to be called (but can be called in parallel with other tasks).

[amrmahdi]

I wonder that maybe we need to instead break the export phase into two passes so that the push can run in parallel, but creating the objects is still sequential.

@tonistiigi i took a stab at it, let me know what you think

[tonistiigi]

I think the approach is ok, check the comments and CI.

This doesn't need to be in the first PR, but if we add Finalize to exporter then all the exporters should use it, not just the image exporter.

[amrmahdi]

I think the approach is ok, check the comments and CI.

This doesn't need to be in the first PR, but if we add Finalize to exporter then all the exporters should use it, not just the image exporter.

Sure, I can addressed the other exporters in a follow up PR

[amrmahdi]

@tonistiigi any chance this can make it to the v0.27.0 release ? 😇

[tonistiigi]

any chance this can make it to the v0.27.0 release ?

Too late for that.

[Copilot]

Pull request overview

This PR reduces overall build latency by parallelizing image finalization (e.g., registry push) with remote cache exports, while preserving the required sequential behavior for inline cache.

Changes:

• Introduces an exporter.FinalizeFunc returned from ExporterInstance.Export to split artifact creation from post-export operations (like pushes).
• Updates the solver to run exporter finalizers and remote cache exporters concurrently via an errgroup.
• Refactors remote cache export execution into a per-exporter function and adjusts existing exporters to the new Export signature.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
solver/llbsolver/solver.go	Runs exporter finalizers and cache exporters in parallel; refactors cache export runner and updates exporter execution flow.
exporter/exporter.go	Extends exporter interface to return an optional finalize callback.
exporter/containerimage/export.go	Moves registry push work into a finalize callback to enable parallelism.
exporter/oci/export.go	Updates exporter signature to return (resp, finalize, descref, err) (finalize is nil).
exporter/local/export.go	Updates exporter signature to include finalize callback (nil).
exporter/tar/export.go	Updates exporter signature and adjusts tar send completion handling (finalize is nil).

Comments suppressed due to low confidence (1)

solver/llbsolver/solver.go:893

• runExporters assigns to the outer named return variable err from inside multiple eg.Go goroutines (resps[i], finalizeFuncs[i], descs[i], err = exp.Export(...)). This is a data race and can also cause the wrong error value to be observed/returned. Use a goroutine-local variable (e.g., resp, fin, desc, expErr := exp.Export(...)) and then assign into the slices, returning expErr without touching the outer err from concurrent goroutines.

				resps[i], finalizeFuncs[i], descs[i], err = exp.Export(ctx, inp, exporter.ExportBuildInfo{
					Ref:         ref,
					SessionID:   job.SessionID,
					InlineCache: inlineCache,
				})
				if err != nil {
					return err
				}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tonistiigi]

I think this is close. Maybe remove the parallelization of multiple --cache-to in the initial PR (see comments).

Also squash the commits. If you have separate logical chunks, then they can be in separate commits, but avoid code that gets reverted later or "review commits" in the version that gets merged. Also, all commits should build.

[tonistiigi]

Potential follow-up in the future would be to push separate registries (or repo names?) in parallel.

[tonistiigi]

Is the parallel cache export really an issue for you? Iiuc if you really have multiple --cache-to and they use different compression conf then the current implementation would be undeterministic.

Also wonder if when multiple --cache-to mode=max then they can try to compete with each other on creating the same layer tarballs in parallel and actually be slower overall.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.