beefsack/go-rate — pull request #3 by matttproud (2014-08-26 02:14:04+00:00)
Title: Fix sundries under the hood to reduce complexity.

The first change is to eliminate the unused wLock mutex. Nothing
uses it, so it is dead code.

The second change is to drop the waitMutex, for Wait piggy backs on
Try; and Try and Try alone mutates the state of the RateLimiter thusly
meaning that waitMutex was a superfluous lock. Users of Wait will see
the same behavior as before, except with less locking.

The third change is to replace the pointerized sync.Mutex with an
inline struct field, which default struct initialization will
correctly make. sync.Mutex operations work on a pointerized receiver
since the Mutex is stateful (http://goo.gl/aKzI2B), thusly meaning our
pointerizing the field is superfluous and further complicates the New
factory. Further, because RateLimiter's method set consists solely of
pointerized method receivers, we pass a reference to a RateLimiter on
each operation, thereby meaning that there is one and one only mutex
per RateLimiter, the one being mutated here. A side benefit of this
is that the field embedding natively in the RateLimiter means less
work for the garbage collector when bookkeeping objects on the heap.
For instance, once RateLimiter is no longer needed, no additional in
the heap need to be performed to determine if the sync.Mutex is still
needed or not.

The fourth change is to replace the times accumulator with a slice
copy to shift the elements forward by one. The current semantic of
shifting the slice view means that---subject to the behavior of the
builting slice append in terms of what new target capacity it leaves
in the newly-returned slice (r.times)---that the append(r.times, foo)
call allocates a new slice when it reaches capacity. By performing the
explicit copy to shift, we decrease allocations and throw away objects
per mutation. See http://play.golang.org/p/y2DzHRv7ZF for a quick
demo.

The fifth change is to bookkeep the tim in Try() once and once only to
ensure consistency in the entries in r.times.

The sixth change is to create r.times as a slice with zero length but
limit capacity. The previous behavior created a slice that effectively
contained zero-valued time.Time elements, which works, but is
semantically incorrect and would confuse users debugging the library.

The seventh change is to drop the superfluous limit field in
RateLimiter since it is implied by the cap() builtin for r.times.
The cap builtin uses the fast path of reading directly from the
internal struct field: http://goo.gl/6CWyPn.