-
Notifications
You must be signed in to change notification settings - Fork 18k
io: implement WriterTo on result of MultiReader #50842
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
Comments
Change https://golang.org/cl/381194 mentions this issue: |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
@rsc thx for the update, I have a patch implementing that, should I wait for the next merge window or I open a PR right now ? |
Please go ahead and send a pull request whenever is convenient for you. It won't be reviewed until after the 1.18 release is out, though. Thanks. |
Change https://go.dev/cl/388455 mentions this issue: |
Fixes golang#50842. Third version, previous: https://golang.org/cl/388455 This patch allows to zerocopy using MultiReader. This is done by MultiReader implementing WriterTo. Each sub reader is copied using usual io copy helper and thus use WriterTo or ReadFrom with reflection. There is a special case for when a subreader is a MultiReader. Instead of using copyBuffer which would call multiReader.WriteTo, multiReader.writeToWithBuffer is used instead, the difference is that the temporary copy buffer is passed along, saving allocations for nested MultiReaders. The workflow looks like this: - multiReader.WriteTo (allocates 32k buffer) - multiReader.writeToWithBuffer - for each subReader: - is instance of multiReader ? - yes, call multiReader.writeToWithBuffer - no, call copyBuffer(writer, currentReader, buffer) - does currentReader implements WriterTo ? - yes, use use currentReader.WriteTo - no, does writer implement ReadFrom ? - yes, use writer.ReadFrom - no, copy using Read / Write with buffer This can be improved by lazy allocating the 32k buffer. For example a MultiReader of such types: MultiReader( bytes.Reader, // WriterTo-able bytes.Reader, // WriterTo-able bytes.Reader, // WriterTo-able ) Doesn't need any allocation, all copy can be done using bytes.Reader's internal data slice. However currently we still allocate a 32k buffer for nothing. This optimisation has been omited for a future patch because of high complexity costs for a non obvious performance cost (it need a benchmark). This patch at least is on par with the previous multiReader.Read workflow allocation wise.
Change https://go.dev/cl/390215 mentions this issue: |
Third version, previous: https://golang.org/cl/388455 This patch allows to zerocopy using MultiReader. This is done by MultiReader implementing WriterTo. Each sub reader is copied using usual io copy helper and thus use WriterTo or ReadFrom with reflection. There is a special case for when a subreader is a MultiReader. Instead of using copyBuffer which would call multiReader.WriteTo, multiReader.writeToWithBuffer is used instead, the difference is that the temporary copy buffer is passed along, saving allocations for nested MultiReaders. The workflow looks like this: - multiReader.WriteTo (allocates 32k buffer) - multiReader.writeToWithBuffer - for each subReader: - is instance of multiReader ? - yes, call multiReader.writeToWithBuffer - no, call copyBuffer(writer, currentReader, buffer) - does currentReader implements WriterTo ? - yes, use use currentReader.WriteTo - no, does writer implement ReadFrom ? - yes, use writer.ReadFrom - no, copy using Read / Write with buffer This can be improved by lazy allocating the 32k buffer. For example a MultiReader of such types: MultiReader( bytes.Reader, // WriterTo-able bytes.Reader, // WriterTo-able bytes.Reader, // WriterTo-able ) Doesn't need any allocation, all copy can be done using bytes.Reader's internal data slice. However currently we still allocate a 32k buffer for nothing. This optimisation has been omited for a future patch because of high complexity costs for a non obvious performance cost (it need a benchmark). This patch at least is on par with the previous multiReader.Read workflow allocation wise. Fixes golang#50842
I have a program where I use
io#MultiReader
to concatenate abytes#Buffer
with a*os#File
.However doing that I loose zero copying on the file.
I added
io#WriterTo
toio#MultiReader
and in my case that makes the copy loop of my app about 120 times faster. I am performing zero-copies of big file, this explain such jump.Anyway I think you shouldn't loose zero copy on
MultiReader
.The text was updated successfully, but these errors were encountered: