Skip to content

Conversation

altenhof
Copy link
Contributor

@altenhof altenhof commented Jan 2, 2017

Should alllow us to pool byte buffers across multiple calls.

Should alllow us to pool byte buffers across multiple calls.
@crewjam
Copy link
Owner

crewjam commented Jan 2, 2017

Thanks for your PR. This looks like it should be named WriteTo and implement the io.WriterTo interface, eh?

@altenhof
Copy link
Contributor Author

altenhof commented Jan 2, 2017

Unfortunately, WriteTo is already taken, defined in stream.go, which, btw, does use Writer and not WriterTo as an argument.

So, what about MarshalTo(w io.Writer)?

@crewjam
Copy link
Owner

crewjam commented Jan 3, 2017

I'm hesitant to use a "wrong" name for a function that has a standard defined interface like WriterTo.

Perhaps a solution is to change the existing stream format WriteTo() to a different name, or attach it to a helper struct. That way the function you wrote can be (correctly) named WriteTo. I think that makes the object behave closer to the way people expect.

@altenhof
Copy link
Contributor Author

altenhof commented Jan 4, 2017

I'm not sure what the "right" name for such a function would be, but maybe I just shortly recap what my initial idea was:

I'm using the library within a component that does "log forwarding", i.e. get log messages in syslog format, transforms them and then forwards them to another syslog endpoint. As I'm expecting fairly heavy loads (several thousand msg/sec), I wanted to have a way to "reuse" bytes.Buffer instances (using sync.Pool) when it comes to forwarding the messages. Right now, MarshalBinary allocate a new bytes.Buffer for every call.

So, I'm not that much looking for a WriteTo function that could write to a io.Writer interface, but for a MarshalTo marshalling the message into a bytes.Buffer. I would have done it "myself", but then I would have to duplicate most of the code in marshal.go.

So how shall we proceed?

@altenhof
Copy link
Contributor Author

Any update on this?

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

Successfully merging this pull request may close these issues.

2 participants