filter/count: Fix alignment on 32-bit systems.

filter/count: Fix alignment on 32-bit systems.

R=iannucci@chromium.org
BUG=

Committed: https://github.com/luci/gae/commit/351e21610f9158e7c4148e5058e97a94107ded8c

[iannucci, 2015-08-22 01:00:56]

https://codereview.chromium.org/1306373002/diff/20001/filter/count/count.go
File filter/count/count.go (right):

https://codereview.chromium.org/1306373002/diff/20001/filter/count/count.go#n...
filter/count/count.go:21: atomic.AddInt64(&c.value, 1)
as discussed offline, let's just change the current implementation to use 32bit
atomics, since they don't have the alignment bug/requirement.

[dnj (Google), 2015-08-22 01:05:44]

On 2015/08/22 01:00:56, iannucci wrote:
> https://codereview.chromium.org/1306373002/diff/20001/filter/count/count.go
> File filter/count/count.go (right):
> 
>
https://codereview.chromium.org/1306373002/diff/20001/filter/count/count.go#n...
> filter/count/count.go:21: atomic.AddInt64(&c.value, 1)
> as discussed offline, let's just change the current implementation to use
32bit
> atomics, since they don't have the alignment bug/requirement.

Done.

[iannucci, 2015-08-22 01:10:51]

lgtm

https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go
File filter/count/count.go (right):

https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go#n...
filter/count/count.go:24: func (c *Counter) Get() int {
I don't think Counter is really necessary, though I suppose you're trying to
prevent slicing an int32 (why not uint32?) in the test (assuming you Get() in
parallel with the usage of the filter, which is probably unlikely to the point
of never :)).

[dnj (Google), 2015-08-22 01:12:54]

On 2015/08/22 01:10:51, iannucci wrote:
> lgtm
> 
> https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go
> File filter/count/count.go (right):
> 
>
https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go#n...
> filter/count/count.go:24: func (c *Counter) Get() int {
> I don't think Counter is really necessary, though I suppose you're trying to
> prevent slicing an int32 (why not uint32?) in the test (assuming you Get() in
> parallel with the usage of the filter, which is probably unlikely to the point
> of never :)).

What I'm trying to prevent is exposing the int32 value publicly, which would
circumvent the atomic barrier (b/c we must use LoadInt32). Alternative is to
implement Errors() and Successes(). WDYT?

[iannucci, 2015-08-22 01:13:53]

On 2015/08/22 01:12:54, dnj (Google) wrote:
> On 2015/08/22 01:10:51, iannucci wrote:
> > lgtm
> > 
> > https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go
> > File filter/count/count.go (right):
> > 
> >
>
https://codereview.chromium.org/1306373002/diff/40001/filter/count/count.go#n...
> > filter/count/count.go:24: func (c *Counter) Get() int {
> > I don't think Counter is really necessary, though I suppose you're trying to
> > prevent slicing an int32 (why not uint32?) in the test (assuming you Get()
in
> > parallel with the usage of the filter, which is probably unlikely to the
point
> > of never :)).
> 
> What I'm trying to prevent is exposing the int32 value publicly, which would
> circumvent the atomic barrier (b/c we must use LoadInt32). Alternative is to
> implement Errors() and Successes(). WDYT?

'sfine as-is

[iannucci, 2015-08-22 01:22:03]

lgtm

[dnj, 2015-08-22 01:23:59]

Committed patchset #4 (id:60001) manually as
351e21610f9158e7c4148e5058e97a94107ded8c (presubmit successful).