write buffer pooling

[y3llowcake]

No description provided.

[garyburd]

Add a test. I suggest adding a loop with pools at

websocket/conn_test.go

Line 80 in 0868951

for _, compress := range []bool{false, true} {

. Follow the pattern for writers. The pools to test are:

• The nil pool.
• A pool that fails if call order is not Get / Put / Get / Put ... Get / Put. Calls must alternate between Get and Put. First call must be Get. Last call must be Put.
• sync.Pool

[garyburd]

Mention that sync.Pool implements the interface. Also state that a shared pool must support concurrent Get() and Put().

[kisielk]

Since this pool interface is specific to this package, shouldn't it work with []byte instead of interface{}? It would require a wrapper around sync.Pool but it seems like it would make the API more specific.

[garyburd]

Working with []byte increases friction for applications that use sync.Pool. How does working with []byte benefit applications?

[garyburd]

I am not enthusiastic about my suggestion to add the dialer and upgrader write pool fields in this PR. As I said in #193, I want to avoid adding new knobs to the API. On the other hand, I don't want an unexpected change in performance for existing users of the package. If anybody has thoughts on this issue, then please comment.

[y3llowcake]

How about we just to expose boolean fields in Upgrader and Dialer. e.g:
type Upgrader struct { ... EnableBufferPooling bool }

This seems to expose fewer details and is arguably less brittle. It could also be reused as the signal for enabling pooling of compression and read buffers when we get around to it.

The tricky bit is managing pools for different buffer sizes internally, but this seems like a tractable problem.

[garyburd]

EnableBufferPooling sounds like a good plan. I suggest calling it EnableWriteBufferPooling in case it becomes possible to also enable read buffer pooling (see golang/go#15735).

How about rounding the buffer size up to the next power of 2 and keeping one pool for each power of 2?

[y3llowcake]

Yah, this sounds reasonable. I'll try the new approach in this PR soon.

In the mean time, I'd appreciate if you could take a look at #194 . I'm hoping to make progress fixing the existing flate performance situation, but hope it will converge with the work in this PR soon.

[AlekSi]

Any news on this one?

[theckman]

@y3llowcake any interest in continuing this PR?

Is there anyone else who is interested in this that wants to pick up the branch and look at resolving the merge conflicts?

[y3llowcake]

Sorry, no. I'm no longer using this library. Good luck.