From ab26eb412646d995bc07af659e8abd4f679ae63d Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Wed, 13 Jan 2021 01:21:28 -0800 Subject: [PATCH] Add WithInversionCache and use pointer methods (#160) There appears to be writes to value receivers. Add `WithInversionCache(bool)` to disable cache. Fixes #159 --- inversion_tree.go | 30 +++++++++++++++++------------- options.go | 17 ++++++++++++++--- reedsolomon.go | 6 ++++-- reedsolomon_test.go | 1 + 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/inversion_tree.go b/inversion_tree.go index c9d8ab2..3f97f81 100644 --- a/inversion_tree.go +++ b/inversion_tree.go @@ -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 } @@ -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() @@ -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 { @@ -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 @@ -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 diff --git a/options.go b/options.go index a8e923c..26269eb 100644 --- a/options.go +++ b/options.go @@ -19,6 +19,7 @@ type options struct { usePAR1Matrix bool useCauchy bool fastOneParity bool + inversionCache bool // stream options concReads bool @@ -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), @@ -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. diff --git a/reedsolomon.go b/reedsolomon.go index e1214d0..cdd320d 100644 --- a/reedsolomon.go +++ b/reedsolomon.go @@ -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 @@ -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 { diff --git a/reedsolomon_test.go b/reedsolomon_test.go index ead6673..2d8953a 100644 --- a/reedsolomon_test.go +++ b/reedsolomon_test.go @@ -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 {