fix(pkg/counter): finish making counter atomic

[rafaeldtinoco]

a6817c1 fix(pkg/counter): finish making counter atomic __{(2023/Jun/27) Rafael David Tinoco <raf>}

Last changes renamed Increment/Decrement to Increase/Decrease. This was
reverted after discussions.

With this new approach:

- Overflow is detected in both cases (and proved by test).
- Thread safety is guaranteed by atomic Add/Sub operations.
- Unit tests prove all assumptions and thread safety for all cases.

NOTE: The issue is now fully fixed...

Originally, the bug was only about reading counter value in parallel to
atomic operation (multiple LOAD/STORE simultaneously, and some not
protected by atomics).

Previous fix, made by commit 1e54ce292, fixed data race but overflow errors
could be reported more than once. It also had atomic operations for each
given argument (if multiple arguments).

This version addresses all concerns.

Heavily influenced by conversations after #3265 (and offline).

[geyslan]

I reviewed and identified some corner cases to be checked. Also suggested a change in the zeroed value tests to contemplate slice of zeroes.

[rafaeldtinoco]

I've included benchmark tests, and removed the extra check for x > 0 that was a leftover. All the rest I believe we are good.

[rafaeldtinoco]

@geyslan Added tests for (0) and (0,0,0). Added benchmarks for all cases:

• Inc()
• Inc(x)
• Inc(0)
• Inc(0,0,0)

The extra cost for checking if val > 0 (about 200ns) is almost 1/2 of the cost of the function entirely (so unless > 50% of the calls of Increment are Increment(0) it wouldn't be worth adding a short circuit).

There is an extra cost of having "argument slice" as well (differences among benchmarks of Inc() and Inc(0) also sums that.

Hope that answers your concerns.

PS: I fixed the extra x > 0 check that was leftover.

Thanks for the review.

NOTE: IF we decide to remove the []uint64 as the argument of the API, I can change this PR.

[NDStrahilevitz]

Simple and more importantly thread-safe, LGTM. Added some styling suggestions but nothing critical.

NOTE: IF we decide to remove the []uint64 as the argument of the API, I can change this PR.

That said, i'm definitely in favor of this.

[yanivagman]

LGTM

already got an approval, lero lero =D