Skip to content

Commit

Permalink
Add WithInversionCache and use pointer methods (#160)
Browse files Browse the repository at this point in the history
There appears to be writes to value receivers.

Add `WithInversionCache(bool)` to disable cache.

Fixes #159
  • Loading branch information
klauspost authored Jan 13, 2021
1 parent 7c86824 commit ab26eb4
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 18 deletions.
30 changes: 17 additions & 13 deletions inversion_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// The tree uses a Reader-Writer mutex to make it thread-safe
// when accessing cached matrices and inserting new ones.
type inversionTree struct {
mutex *sync.RWMutex
mutex sync.RWMutex
root inversionNode
}

Expand All @@ -26,21 +26,22 @@ type inversionNode struct {
// newInversionTree initializes a tree for storing inverted matrices.
// Note that the root node is the identity matrix as it implies
// there were no errors with the original data.
func newInversionTree(dataShards, parityShards int) inversionTree {
func newInversionTree(dataShards, parityShards int) *inversionTree {
identity, _ := identityMatrix(dataShards)
root := inversionNode{
matrix: identity,
children: make([]*inversionNode, dataShards+parityShards),
}
return inversionTree{
mutex: &sync.RWMutex{},
root: root,
return &inversionTree{
root: inversionNode{
matrix: identity,
children: make([]*inversionNode, dataShards+parityShards),
},
}
}

// GetInvertedMatrix returns the cached inverted matrix or nil if it
// is not found in the tree keyed on the indices of invalid rows.
func (t inversionTree) GetInvertedMatrix(invalidIndices []int) matrix {
func (t *inversionTree) GetInvertedMatrix(invalidIndices []int) matrix {
if t == nil {
return nil
}
// Lock the tree for reading before accessing the tree.
t.mutex.RLock()
defer t.mutex.RUnlock()
Expand All @@ -63,7 +64,10 @@ var errAlreadySet = errors.New("the root node identity matrix is already set")
// keyed by the indices of invalid rows. The total number of shards
// is required for creating the proper length lists of child nodes for
// each node.
func (t inversionTree) InsertInvertedMatrix(invalidIndices []int, matrix matrix, shards int) error {
func (t *inversionTree) InsertInvertedMatrix(invalidIndices []int, matrix matrix, shards int) error {
if t == nil {
return nil
}
// If no invalid indices were given then we are done because the
// root node is already set with the identity matrix.
if len(invalidIndices) == 0 {
Expand All @@ -86,7 +90,7 @@ func (t inversionTree) InsertInvertedMatrix(invalidIndices []int, matrix matrix,
return nil
}

func (n inversionNode) getInvertedMatrix(invalidIndices []int, parent int) matrix {
func (n *inversionNode) getInvertedMatrix(invalidIndices []int, parent int) matrix {
// Get the child node to search next from the list of children. The
// list of children starts relative to the parent index passed in
// because the indices of invalid rows is sorted (by default). As we
Expand Down Expand Up @@ -117,7 +121,7 @@ func (n inversionNode) getInvertedMatrix(invalidIndices []int, parent int) matri
return node.matrix
}

func (n inversionNode) insertInvertedMatrix(invalidIndices []int, matrix matrix, shards, parent int) {
func (n *inversionNode) insertInvertedMatrix(invalidIndices []int, matrix matrix, shards, parent int) {
// As above, get the child node to search next from the list of children.
// The list of children starts relative to the parent index passed in
// because the indices of invalid rows is sorted (by default). As we
Expand Down
17 changes: 14 additions & 3 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type options struct {
usePAR1Matrix bool
useCauchy bool
fastOneParity bool
inversionCache bool

// stream options
concReads bool
Expand All @@ -27,9 +28,10 @@ type options struct {
}

var defaultOptions = options{
maxGoroutines: 384,
minSplitSize: -1,
fastOneParity: false,
maxGoroutines: 384,
minSplitSize: -1,
fastOneParity: false,
inversionCache: true,

// Detect CPU capabilities.
useSSSE3: cpuid.CPU.Supports(cpuid.SSSE3),
Expand Down Expand Up @@ -109,6 +111,15 @@ func WithConcurrentStreamWrites(enabled bool) Option {
}
}

// WithInversionCache allows to control the inversion cache.
// This will cache reconstruction matrices so they can be reused.
// Enabled by default.
func WithInversionCache(enabled bool) Option {
return func(o *options) {
o.inversionCache = enabled
}
}

// WithStreamBlockSize allows to set a custom block size per round of reads/writes.
// If not set, any shard size set with WithAutoGoroutines will be used.
// If WithAutoGoroutines is also unset, 4MB will be used.
Expand Down
6 changes: 4 additions & 2 deletions reedsolomon.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type reedSolomon struct {
ParityShards int // Number of parity shards, should not be modified.
Shards int // Total number of shards. Calculated, and should not be modified.
m matrix
tree inversionTree
tree *inversionTree
parity [][]byte
o options
mPool sync.Pool
Expand Down Expand Up @@ -333,7 +333,9 @@ func New(dataShards, parityShards int, opts ...Option) (Encoder, error) {
// The inversion root node will have the identity matrix as
// its inversion matrix because it implies there are no errors
// with the original data.
r.tree = newInversionTree(dataShards, parityShards)
if r.o.inversionCache {
r.tree = newInversionTree(dataShards, parityShards)
}

r.parity = make([][]byte, parityShards)
for i := range r.parity {
Expand Down
1 change: 1 addition & 0 deletions reedsolomon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func testOpts() [][]Option {
{WithMaxGoroutines(5000), WithMinSplitSize(500000), withSSSE3(false), withAVX2(false), withAVX512(false)},
{WithMaxGoroutines(1), WithMinSplitSize(500000), withSSSE3(false), withAVX2(false), withAVX512(false)},
{WithAutoGoroutines(50000), WithMinSplitSize(500)},
{WithInversionCache(false)},
}
for _, o := range opts[:] {
if defaultOptions.useSSSE3 {
Expand Down

0 comments on commit ab26eb4

Please sign in to comment.