Add bufferpool, a sync.Pool-backed pool of buffers

Add bufferpool, a sync.Pool-backed pool of buffers

BUG=chromium:675485
TEST=unit

Review-Url: https://codereview.chromium.org/2596633002
Committed: https://github.com/luci/luci-go/commit/c5d59a001263b51aeae9d033de99bb2598845603

[dnj, 2016-12-20 22:29:57]

dnj@chromium.org changed reviewers:
+ vadimsh@chromium.org

[dnj, 2016-12-20 22:29:57]

PTAL

[Vadim Sh., 2016-12-20 23:11:51]

lgtm

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
File common/sync/bufferpool/buffer_pool.go (right):

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
common/sync/bufferpool/buffer_pool.go:62: // After calling Release, no accesses
may be made to b or its internal data.
you can ensure this by setting b.p and b.Buffer to nil, so any access would
panic.

[Vadim Sh., 2016-12-20 23:13:51]

actually, not yet

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
File common/sync/bufferpool/buffer_pool.go (right):

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
common/sync/bufferpool/buffer_pool.go:62: // After calling Release, no accesses
may be made to b or its internal data.
On 2016/12/20 23:11:51, Vadim Sh. wrote:
> you can ensure this by setting b.p and b.Buffer to nil, so any access would
> panic.

Hm... For that, sync.Pool must hold referenced to bytes.Buffer{}, not
bufferpool.Buffer.

I think it might be better anyhow. I'm not sure sync.Pool GC magic is capable of
dealing with circular reference (pooled object keeping a pointer to the pool).

[dnj, 2016-12-21 00:54:38]

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
File common/sync/bufferpool/buffer_pool.go (right):

https://codereview.chromium.org/2596633002/diff/20001/common/sync/bufferpool/...
common/sync/bufferpool/buffer_pool.go:62: // After calling Release, no accesses
may be made to b or its internal data.
On 2016/12/20 23:13:51, Vadim Sh. wrote:
> On 2016/12/20 23:11:51, Vadim Sh. wrote:
> > you can ensure this by setting b.p and b.Buffer to nil, so any access would
> > panic.
> 
> Hm... For that, sync.Pool must hold referenced to bytes.Buffer{}, not
> bufferpool.Buffer.
> 
> I think it might be better anyhow. I'm not sure sync.Pool GC magic is capable
of
> dealing with circular reference (pooled object keeping a pointer to the pool).

Done.

[Vadim Sh., 2016-12-21 01:01:14]

lgtm

[dnj, 2016-12-21 01:02:30]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2016-12-21 01:02:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-21 01:08:11]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482282150468440,
"parent_rev": "72a7572c413200844a2429d482c41bb863557aa6", "commit_rev":
"c5d59a001263b51aeae9d033de99bb2598845603"}

[commit-bot: I haz the power, 2016-12-21 01:08:14]

Description was changed from

==========
Add bufferpool, a sync.Pool-backed pool of buffers

BUG=chromium:675485
TEST=unit
==========

to

==========
Add bufferpool, a sync.Pool-backed pool of buffers

BUG=chromium:675485
TEST=unit

Review-Url: https://codereview.chromium.org/2596633002
Committed:
https://github.com/luci/luci-go/commit/c5d59a001263b51aeae9d033de99bb2598845603
==========

[commit-bot: I haz the power, 2016-12-21 01:08:15]

Committed patchset #4 (id:60001) as
https://github.com/luci/luci-go/commit/c5d59a001263b51aeae9d033de99bb2598845603