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
proposal: io: CopierTo and CopierFrom interfaces #67074
Comments
Is CopyTo guaranteed to have had no observable effect when it returns an errors.Is(,errors.ErrUnsupported) error? |
Yes. |
Maybe I'm missing the point but what precisely is the issue with solving this how you opt out of any unwanted type unerasure by forcing a barrier struct (as already mentioned in #16474)? If you are using io.CopyBuffer, chances are you specially prepared for the buffer, so we are talking about a deliberate, pin-pointed optimization, not implicit system-wide optimizations which io.WriterTo do help with. io.CopyBuffer is not alone in exhibiting behaviour not expressible in the type system which one needs to be wary about: compress/zlib.NewReader may discard data if the reader cannot unerase to an exact one and bufio.(*Reader).WriteTo can even call ReadFrom (although undocumented). If you want to prohibit a function from utilizing its fast paths, hide your own capabilities. |
The motivation for this proposal is to fix It's perhaps not immediately obvious that func (t *transferWriter) doBodyCopy(dst io.Writer, src io.Reader) (n int64, err error) {
buf := getCopyBuf()
defer putCopyBuf(buf)
n, err = io.CopyBuffer(dst, src, buf)
if err != nil && err != io.EOF {
t.bodyReadError = err
}
return
} This function copies from a user-provided Let's consider the case where we're on macOS, copying from an In this case:
Note that there are three We could change This is a mess. At a high level, I have two goals. When using common types from the standard library (files, network sockets):
Arguably, the recursive calls to The Go compatibility promise closes off most avenues for fixing this:
(Of these options, the last one seems like the most feasible: If could return The root of the problem is that
The proposed Adding yet more magic methods to dig us out of the hole caused by the existing magic methods does seem somewhat odd. It's quite possible that if we were designing |
We could constrain this by using an internal package, and by looking for methods that return types defined in that internal package. That would mean that only types in the standard library would implement the new interfaces. On the one hand that would prevent other packages from taking advantage of these new methods. On the other hand it would constrain the additional complexity. For example package iofd
type FD uintptr
type SysFDer interface {
SysFD() FD
CopyBetween(dst, src FD, buf []byte) (int64, bool, error)
} Then in package io srcfd, srcOK := src.(iofd.SysFDer)
dstfd, dstOK := dst.(iofd.SysFDer)
if srcOK && dstOK {
len, handled, err := srcfd.CopyBetween(dstfd, srcfd, buf)
if handled {
return len, err
}
} |
We could constrain this to be internal-only, but I don't think it's worth it. We'd still have the complexity of the new methods, we'd just not be allowing anyone else to take advantage of the benefits. |
I think the complexity of the new methods is significantly less if nobody else can implement them. In particular we can design the new methods so that they work on the pair of source and destination, which is what we need. One of the problems with the current methods is that they only work on the source or on the destination, which is how we've gotten into the position of needing to use both, and therefore needing to turn off both, and therefore increasing complexity. |
Proposal Details
The
io.Copy
andio.CopyBuffer
functions will useWriteTo
when the source supports it andReadFrom
when the destination supports it.There are cases when a type can efficiently support
WriteTo
orReadFrom
, but only some of the time. For example, CL 472475 added anWriteTo
method to*os.File
which performs a more efficient copy on Linux systems when the destination is a Unix or TCP socket. In all other cases, it falls back toio.Copy
.The
io.Copy
fallback means thatio.CopyBuffer
with an*os.File
source will no longer use the provided buffer, because the buffer is not plumbed through toWriteTo
. (It also resulted in https://go.dev/issue/66988, in which the fallback resulted in the fast path in *net.TCPConn.ReadFrom not being taken.)There have been previous proposals to fix this problem:
io.Copy
case.Those issues also contain some links to various other instances of this problem turning up, but I think the
*os.File.WriteTo
case is a sufficient example of a problem that needs solving, sinceio.CopyBuffer
no longer works as expected on files.I propose that we add two new interfaces to the
io
package:The
CopierTo
andCopierFrom
interfaces supersedeWriterTo
andReaderFrom
when available. They provide a way to plumb the copy buffer down through the copy operation, and permit an implementation to implement a fast path for some subset of sources or destinations while deferring toio.Copy
for other cases.We will update
io.Copy
andio.CopyBuffer
to preferCopierTo
andCopierFrom
when available.We will update
*os.File
and*net.TCPConn
(and possibly other types) to addCopyTo
andCopyFrom
methods.The text was updated successfully, but these errors were encountered: