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

Add proxywriter #52

Closed
donny-dont opened this issue Oct 16, 2019 · 3 comments
Closed

Add proxywriter #52

donny-dont opened this issue Oct 16, 2019 · 3 comments

Comments

@donny-dont
Copy link

The io.Copy function has optimizations where the Reader implements io.ReadFrom.

// copyBuffer is the actual implementation of Copy and CopyBuffer.
// if buf is nil, one is allocated.
func copyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
	// If the reader has a WriteTo method, use it to do the copy.
	// Avoids an allocation and a copy.
	if wt, ok := src.(WriterTo); ok {
		return wt.WriteTo(dst)
	}
	// Similarly, if the writer has a ReadFrom method, use it to do the copy.
	if rt, ok := dst.(ReaderFrom); ok {
		return rt.ReadFrom(src)
	}
        ....

For example sftp.File implements io.ReadFrom. So if you attempt to download a file from sftp and wrap it with a mpb.proxyReader then io.Copy is unable to use the io.ReaderFrom optimization.

I was thinking about migrating from https://github.com/cheggaaa/pb to mpb since it has multiple progress bar support and noticed that mpb just proxies a reader. Would it be of interest to do a writer proxy as well?

@vbauerster
Copy link
Owner

Hello!
Thanks for pointing to io.ReadFrom, indeed it wasn't accounted for. Commit 9436aa1 should fix this. If you have time and interest to implement a proxy for writer side, I'd be glad to review a PR.
You mentioned sftp.File which is a good use case for proxy reader, can you think of any use case for proxy writer?

@nektro
Copy link

nektro commented Nov 28, 2019

I've done this externally in a library I wrote to wrap mpb. Most relavant parts:
are here: https://github.com/nektro/go-util/blob/master/mbpp/mbpp.go#L144
and here: https://github.com/nektro/go-util/blob/master/mbpp/headlessbar.go

@vbauerster
Copy link
Owner

I think the title of the issue is a bit misleading, because @donny-dont asked for io.WriterTo interface to be implemented, so that io.Copy can benefit, if used with internal proxy reader. As this is done I'm closing this issue.

@nektro thanks for interesting code, anyway.

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

3 participants