Skip to content

Conversation

@dmcgowan
Copy link
Member

Allows optimized copying from a local content file into another file.

Using a *LimitedReader allows for an optimized copy file range to be performed when copying into a *os.File on Linux. Currently our custom type or use of section reader hide the source *os.File.
See https://cs.opensource.google/go/go/+/refs/tags/go1.19.1:src/os/readfrom_linux.go;drc=f1b7b2fc52947711b8e78f7078c9e0bda35320d3;l=14

Allows optimized copying from a local content file into another file.

Signed-off-by: Derek McGowan <[email protected]>
@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@dmcgowan dmcgowan marked this pull request as ready for review September 21, 2022 03:59
func NewReader(ra ReaderAt) io.Reader {
rd := io.NewSectionReader(ra, 0, ra.Size())
return rd
if rd, ok := ra.(reader); ok {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if rd, ok := ra.(reader); ok {
if rd, ok := ra.(io.Reader); ok {

Copy link
Member Author

Choose a reason for hiding this comment

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

io.Reader doesn't help here since the instance cannot be type asserted to *os.File. It needs a new reader instance and ideally carrying the size data, which *LimitedReader does.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit de21641 into containerd:main Sep 22, 2022
Copy link
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants