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

converter: improve pack performance by fifo #44

Merged
merged 2 commits into from
May 9, 2022

Conversation

imeoer
Copy link
Collaborator

@imeoer imeoer commented Apr 29, 2022

In the previous convert implementation, the builder (nydus-image) would
dump the blob and bootstrap, and then the converter would pack the two
files into a tar format, which has a performance loss, this patch solves this
problem by introducing fifo file.

The converter creates a fifo file, the builder will write the blob and
bootstrap data to the fifo as the writer side, then the converter read
from the fifo and write directly to content store for faster conversion.

Related PR: dragonflyoss/nydus#415

Signed-off-by: Yan Song [email protected]

@imeoer imeoer force-pushed the converter-tar-pack branch 2 times, most recently from a8db324 to f8e5006 Compare April 29, 2022 10:38
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2022

Codecov Report

Merging #44 (7621e81) into main (6112f67) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #44   +/-   ##
=======================================
  Coverage   35.09%   35.09%           
=======================================
  Files          18       18           
  Lines        1288     1288           
=======================================
  Hits          452      452           
  Misses        765      765           
  Partials       71       71           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6112f67...7621e81. Read the comment docs.

@imeoer imeoer force-pushed the converter-tar-pack branch from 396d2d6 to 7621e81 Compare April 29, 2022 12:12
@imeoer
Copy link
Collaborator Author

imeoer commented Apr 29, 2022

Wait for this PR dragonflyoss/nydus#415 to be merged then pass the CI testing.

@imeoer imeoer force-pushed the converter-tar-pack branch from 7621e81 to 8f253d0 Compare May 6, 2022 09:57
imeoer added 2 commits May 6, 2022 10:14
In the previous convert implementation, the builder (nydus-image) would
dump the blob and bootstrap, and then the converter would pack the two
files into a tar format, which has a performance loss, and the patch
solves this problem by introducing fifo file.

The converter creates a fifo file, the builder will write the blob and
bootstrap data to the fifo as the writer side, then the converter read
from the fifo and write directly to content store for faster conversion.

Signed-off-by: Yan Song <[email protected]>
The latest nydus static release url has been changed.

Signed-off-by: Yan Song <[email protected]>
@imeoer imeoer force-pushed the converter-tar-pack branch from 8f253d0 to e413bec Compare May 6, 2022 10:14
Copy link
Contributor

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise lgtm!

}
defer blobFifo.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blobFifo is closed upon convert error.

@bergwolf bergwolf merged commit b4c0508 into containerd:main May 9, 2022
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.

4 participants