Skip to content

Add generic buffer.TypedRingGrowing and shrinkable buffer.Ring #323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 98 additions & 14 deletions buffer/ring_growing.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,54 @@ limitations under the License.

package buffer

// RingGrowingOptions sets parameters for [RingGrowing] and
// [TypedRingGrowing].
type RingGrowingOptions struct {
// InitialSize is the number of pre-allocated elements in the
// initial underlying storage buffer.
InitialSize int
}

// RingGrowing is a growing ring buffer.
// Not thread safe.
type RingGrowing struct {
data []interface{}
//
// Deprecated: Use TypedRingGrowing[any] instead.
type RingGrowing = TypedRingGrowing[any]

// NewRingGrowing constructs a new RingGrowing instance with provided parameters.
//
// Deprecated: Use NewTypedRingGrowing[any] instead.
func NewRingGrowing(initialSize int) *RingGrowing {
return NewTypedRingGrowing[any](RingGrowingOptions{InitialSize: initialSize})
}

// TypedRingGrowing is a growing ring buffer.
// The zero value has an initial size of 0 and is ready to use.
// Not thread safe.
type TypedRingGrowing[T any] struct {
data []T
n int // Size of Data
beg int // First available element
readable int // Number of data items available
}

// NewRingGrowing constructs a new RingGrowing instance with provided parameters.
func NewRingGrowing(initialSize int) *RingGrowing {
return &RingGrowing{
data: make([]interface{}, initialSize),
n: initialSize,
// NewTypedRingGrowing constructs a new TypedRingGrowing instance with provided parameters.
func NewTypedRingGrowing[T any](opts RingGrowingOptions) *TypedRingGrowing[T] {
return &TypedRingGrowing[T]{
data: make([]T, opts.InitialSize),
n: opts.InitialSize,
}
}

// ReadOne reads (consumes) first item from the buffer if it is available, otherwise returns false.
func (r *RingGrowing) ReadOne() (data interface{}, ok bool) {
func (r *TypedRingGrowing[T]) ReadOne() (data T, ok bool) {
if r.readable == 0 {
return nil, false
return
}
r.readable--
element := r.data[r.beg]
r.data[r.beg] = nil // Remove reference to the object to help GC
var zero T
r.data[r.beg] = zero // Remove reference to the object to help GC
if r.beg == r.n-1 {
// Was the last element
r.beg = 0
Expand All @@ -51,11 +74,14 @@ func (r *RingGrowing) ReadOne() (data interface{}, ok bool) {
}

// WriteOne adds an item to the end of the buffer, growing it if it is full.
func (r *RingGrowing) WriteOne(data interface{}) {
func (r *TypedRingGrowing[T]) WriteOne(data T) {
if r.readable == r.n {
// Time to grow
newN := r.n * 2
newData := make([]interface{}, newN)
if newN == 0 {
newN = 1
}
newData := make([]T, newN)
to := r.beg + r.readable
if to <= r.n {
copy(newData, r.data[r.beg:to])
Expand All @@ -72,11 +98,69 @@ func (r *RingGrowing) WriteOne(data interface{}) {
}

// Len returns the number of items in the buffer.
func (r *RingGrowing) Len() int {
func (r *TypedRingGrowing[T]) Len() int {
return r.readable
}

// Cap returns the capacity of the buffer.
func (r *RingGrowing) Cap() int {
func (r *TypedRingGrowing[T]) Cap() int {
return r.n
}

// RingGrowingOptions sets parameters for [Ring].
type RingOptions struct {
// InitialSize is the number of pre-allocated elements in the
// initial underlying storage buffer.
InitialSize int
// NormalSize is the number of elements to allocate for new storage
// buffers once the Ring is consumed.
NormalSize int
}

// Ring is a dynamically-sized ring buffer which can grow and shrink as-needed.
// The zero value has an initial size and normal size of 0 and is ready to use.
// Not thread safe.
type Ring[T any] struct {
growing TypedRingGrowing[T]
normalSize int // Limits the size of the buffer that is kept for reuse. Read-only.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queue.FIFO is set up to not require any constructors like this, so users can initialize one directly as the zero value and get a valid, empty queue. One detail I couldn't quite iron out to enable that here is how to make the normalSize configurable. There it's hardcoded to 4 which seems reasonable for where that's used, but I see informers' notification buffers using RingGrowing are initialized to 1024, so shrinking all the way down to 4 could result in significantly more allocations. So if there isn't a single golden value for every use case, I think that value probably has to be fed in through a constructor (or possibly a method).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that a constructor parameter would be good. Two int parameters with no clear obvious ordering will be hard to read at call sites, so I prefer the config struct pattern.

We could use optional With* methods, but that raises the question whether they return a deep copy of the instance or completely new ones - let's not go there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the zero value Ring[T]: let's document it as NewRing() with zero initial size and zero normal size. Perhaps not very useful, but at least it's then documented.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added options structs and mentioned zero values in doc strings.

}

// NewRing constructs a new Ring instance with provided parameters.
func NewRing[T any](opts RingOptions) *Ring[T] {
return &Ring[T]{
growing: *NewTypedRingGrowing[T](RingGrowingOptions{InitialSize: opts.InitialSize}),
normalSize: opts.NormalSize,
}
}

// ReadOne reads (consumes) first item from the buffer if it is available,
// otherwise returns false. When the buffer has been totally consumed and has
// grown in size, it shrinks down to its initial size.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// grown in size, it shrinks down to its initial size.
// grown in size beyond its normal size, it shrinks down to its normal size again.

I was wondering whether two different options are needed, but I can imaging that RingOptions{InitialSize: 8, NormalSize: 32} could make sense: we get some memory reuse for small problems with less than 8 entries max. If the problem space is larger, we hang on to more memory to deal with it.

func (r *Ring[T]) ReadOne() (data T, ok bool) {
element, ok := r.growing.ReadOne()

if r.growing.readable == 0 && r.growing.n > r.normalSize {
// The buffer is empty. Reallocate a new buffer so the old one can be
// garbage collected.
r.growing.data = make([]T, r.normalSize)
r.growing.n = r.normalSize
r.growing.beg = 0
}

return element, ok
}

// WriteOne adds an item to the end of the buffer, growing it if it is full.
func (r *Ring[T]) WriteOne(data T) {
r.growing.WriteOne(data)
}

// Len returns the number of items in the buffer.
func (r *Ring[T]) Len() int {
return r.growing.Len()
}

// Cap returns the capacity of the buffer.
func (r *Ring[T]) Cap() int {
return r.growing.Cap()
}
172 changes: 145 additions & 27 deletions buffer/ring_growing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,45 +17,163 @@ limitations under the License.
package buffer

import (
"reflect"
"testing"
)

func TestGrowth(t *testing.T) {
func TestGrowthGrowing(t *testing.T) {
t.Parallel()
x := 10
g := NewRingGrowing(1)
for i := 0; i < x; i++ {
if e, a := i, g.readable; !reflect.DeepEqual(e, a) {
t.Fatalf("expected equal, got %#v, %#v", e, a)
}
g.WriteOne(i)
}
read := 0
for g.readable > 0 {
v, ok := g.ReadOne()
if !ok {
t.Fatal("expected true")
}
if read != v {
t.Fatalf("expected %#v==%#v", read, v)
}
read++
tests := map[string]struct {
ring *TypedRingGrowing[int]
initialSize int
}{
"implicit-zero": {
ring: new(TypedRingGrowing[int]),
},
"explicit-zero": {
ring: NewTypedRingGrowing[int](RingGrowingOptions{InitialSize: 0}),
initialSize: 0,
},
"nonzero": {
ring: NewTypedRingGrowing[int](RingGrowingOptions{InitialSize: 1}),
initialSize: 1,
},
}
if x != read {
t.Fatalf("expected to have read %d items: %d", x, read)

for name, test := range tests {
t.Run(name, func(t *testing.T) {
initialSize := test.initialSize
g := test.ring

if expected, actual := 0, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := initialSize, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}

x := 10
for i := 0; i < x; i++ {
if e, a := i, g.readable; e != a {
t.Fatalf("expected equal, got %#v, %#v", e, a)
}
g.WriteOne(i)
}

if expected, actual := x, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := 16, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}

read := 0
for g.readable > 0 {
v, ok := g.ReadOne()
if !ok {
t.Fatal("expected true")
}
if read != v {
t.Fatalf("expected %#v==%#v", read, v)
}
read++
}
if x != read {
t.Fatalf("expected to have read %d items: %d", x, read)
}
if expected, actual := 0, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := 16, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}
})
}
if g.readable != 0 {
t.Fatalf("expected readable to be zero: %d", g.readable)

}

func TestGrowth(t *testing.T) {
t.Parallel()

tests := map[string]struct {
ring *Ring[int]
initialSize int
normalSize int
}{
"implicit-zero": {
ring: new(Ring[int]),
},
"explicit-zero": {
ring: NewRing[int](RingOptions{InitialSize: 0, NormalSize: 0}),
initialSize: 0,
normalSize: 0,
},
"smaller-initial-size": {
ring: NewRing[int](RingOptions{InitialSize: 1, NormalSize: 2}),
initialSize: 1,
normalSize: 2,
},
"smaller-normal-size": {
ring: NewRing[int](RingOptions{InitialSize: 2, NormalSize: 1}),
initialSize: 2,
normalSize: 1,
},
}
if 16 != g.n {
t.Fatalf("expected N to be 16: %d", g.n)

for name, test := range tests {
t.Run(name, func(t *testing.T) {
initialSize := test.initialSize
normalSize := test.normalSize
g := test.ring

if expected, actual := 0, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := initialSize, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}

x := 10
for i := 0; i < x; i++ {
if e, a := i, g.growing.readable; e != a {
t.Fatalf("expected equal, got %#v, %#v", e, a)
}
g.WriteOne(i)
}

if expected, actual := x, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := 16, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}

read := 0
for g.growing.readable > 0 {
v, ok := g.ReadOne()
if !ok {
t.Fatal("expected true")
}
if read != v {
t.Fatalf("expected %#v==%#v", read, v)
}
read++
}
if x != read {
t.Fatalf("expected to have read %d items: %d", x, read)
}
if expected, actual := 0, g.Len(); expected != actual {
t.Fatalf("expected Len to be %d, got %d", expected, actual)
}
if expected, actual := normalSize, g.Cap(); expected != actual {
t.Fatalf("expected Cap to be %d, got %d", expected, actual)
}
})
}
}

func TestEmpty(t *testing.T) {
t.Parallel()
g := NewRingGrowing(1)
g := NewTypedRingGrowing[struct{}](RingGrowingOptions{InitialSize: 1})
_, ok := g.ReadOne()
if ok != false {
t.Fatal("expected false")
Expand Down
Loading