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

encoding/binary: Write is too slow #2634

Closed
bradfitz opened this issue Dec 29, 2011 · 10 comments
Closed

encoding/binary: Write is too slow #2634

bradfitz opened this issue Dec 29, 2011 · 10 comments

Comments

@bradfitz
Copy link
Contributor

In my VNC server (where the client picks the endianness), I was trying to use
encoding/binary.Write to write a slice of 921,600 uint16s, 30 times per second.

After being CPU-bound, I looked at a profile and it was dominated by reflect calls.

We've optimized the common cases in encoding/binary, but slices are not optimized at all.

It's also very alloc-heavy, creating a new encoder & scratch space for each Write
call.

I ended up rewriting that part of my code to not use the encoding/binary package and now
it's fast, never stuck in reflect.
@rsc
Copy link
Contributor

rsc commented Jan 9, 2012

Comment 1:

It's not really supposed to be fast, just convenient.
But it could be made faster.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@DanielMorsing
Copy link
Contributor

Comment 2:

I made a simple patch that does a type assertion if the slice is of basic type. This
gets around reflecting on every element and gives a nice speedup
I made a little benchmark included in the patch which just does a null write with some
slices
Before:
BenchmarkWriteSlice1000Int32s      10000        122838 ns/op      32.56 MB/s
After:
BenchmarkWriteSlice1000Int32s     100000         24105 ns/op     165.93 MB/s

Attachments:

  1. binary.patch (2076 bytes)

@bradfitz
Copy link
Contributor Author

Comment 3:

Thanks! We don't use the bug tracker for code submissions or reviews, though.
To contribute code, see:
   http://golang.org/doc/contribute.html

@davecheney
Copy link
Contributor

Comment 5:

Hi, 
If you are still interested in contributing this patch, please follow the procedure here
 http://golang.org/doc/contribute.html
Cheers
Dave

Status changed to WaitingForReply.

@robpike
Copy link
Contributor

robpike commented Nov 2, 2012

Comment 6:

Daniel: Please turn this into a code review.

@davecheney
Copy link
Contributor

Comment 7:

Hi DMorsing, if this patch still makes sense can you please hg mail it, otherwise close
this issue.

Owner changed to @DanielMorsing.

Status changed to Started.

@DanielMorsing
Copy link
Contributor

Comment 8:

I had completely forgotten about this.
The patch is a bit of a hack and with go1.1 coming up, I'm a bit hesitant to make a cl
out of this.

@bradfitz
Copy link
Contributor Author

Comment 9:

Labels changed: added performance.

@DanielMorsing
Copy link
Contributor

Comment 10:

I thought i made a CL out of the original patch and it got rejected because it was too
ugly. In hindsight, I concur with that opinion.
Can't find any mail evidence the rejection though, so maybe my memory is faulty. In any
case, i'm releasing this bug so that other people can work on it.

Owner changed to ---.

Status changed to Accepted.

@robpike
Copy link
Contributor

robpike commented Aug 9, 2013

Comment 11:

This issue was closed by revision c0465d0.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants