Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge+Diff follow-ups #2548

Open
3 of 10 tasks
sipsma opened this issue Jan 6, 2022 · 2 comments
Open
3 of 10 tasks

Merge+Diff follow-ups #2548

sipsma opened this issue Jan 6, 2022 · 2 comments

Comments

@sipsma
Copy link
Collaborator

sipsma commented Jan 6, 2022

Tracking followups from comments on merge+diff here. Can instead break this into separate issues for each one if preferred.

Short-term followups:

Performance followups:

  • Deduplicate equivalent merges
  • Deduplicate equivalent diffs
  • Re-use submerges when possible, i.e. if Merge(a,b) already exists, use it to speed up creation of Merge(a,b,c)
  • Normalize merges to their simplest form, i.e. Merge(a,b,a) is logically equivalent to the simpler Merge(b,a)
  • Don't finalize input refs when they are created by llb.Local. Comment here.

Longer-term followups:

  • Cache diff metadata and use it to apply diffs instead of always running the double-walking or overlay differ every time. Comment here
  • Deduplicate code between diffApply and the containerd/continuity appliers+differs. Comment here.
@sipsma
Copy link
Collaborator Author

sipsma commented Jan 6, 2022

@tonistiigi I will prioritize the short-term follow-ups, especially progress+inline cache+docs since I'm assuming they are hard requirements for a v0.10. Let me know what feelings you have on the priority of the others, especially which ones you consider blocking for the next release.

@sipsma sipsma mentioned this issue Jan 6, 2022
@tonistiigi
Copy link
Member

Support returning direct nil from solver op execs.

I think this is not needed unless you see benefits. It's somewhat nice to validate the return array length like you described in that response.

Don't finalize input refs when they are created by llb.Local

Just a thought as well. I think we need to first understand if there are cases where it is beneficial.

I think the blockers on my side are the progress(I think we need different approach than the draft pr), inline cache(draft PR mostly looks good) and Moby vendor.

I'll try to pick up the work on the Dockerfile side. At least for mergeop, not sure if we are ready for diffop yet. Having some optimizations for copy as like discussed would be nice as well(storing diff metadata on fileop and using direct hardlinks from overlay lowers for copy when possible).

@tonistiigi tonistiigi added this to the v0.10.0 milestone Jan 24, 2022
@crazy-max crazy-max modified the milestones: v0.10.0, v0.12.0 Jan 29, 2023
@jedevc jedevc modified the milestones: v0.12.0, v0.13 Jun 29, 2023
@tonistiigi tonistiigi removed this from the v0.13.0 milestone Feb 12, 2024
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

No branches or pull requests

4 participants