Skip to content

Commit

Permalink
Merge pull request #316 from onflow/fxamacker/change-array-get-to-ret…
Browse files Browse the repository at this point in the history
…urn-value

Change `Array.Get()` to return `Value` instead of `Storable`
  • Loading branch information
fxamacker authored Jun 29, 2023
2 parents f54dc2f + 044db01 commit 0c73dea
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 20 deletions.
15 changes: 12 additions & 3 deletions array.go
Original file line number Diff line number Diff line change
Expand Up @@ -1995,9 +1995,18 @@ func NewArrayWithRootID(storage SlabStorage, rootID StorageID) (*Array, error) {
}, nil
}

func (a *Array) Get(i uint64) (Storable, error) {
// Don't need to wrap error as external error because err is already categorized by ArraySlab.Get().
return a.root.Get(a.Storage, i)
func (a *Array) Get(i uint64) (Value, error) {
storable, err := a.root.Get(a.Storage, i)
if err != nil {
// Don't need to wrap error as external error because err is already categorized by ArraySlab.Get().
return nil, err
}
v, err := storable.StoredValue(a.Storage)
if err != nil {
// Wrap err as external error (if needed) because err is returned by Storable interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get storable's stored value")
}
return v, nil
}

func (a *Array) Set(index uint64, value Value) (Storable, error) {
Expand Down
13 changes: 7 additions & 6 deletions array_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import (
"github.com/stretchr/testify/require"
)

var noop Storable
var noopValue Value
var noopStorable Storable

func BenchmarkArrayGet100x(b *testing.B) {
benchmarks := []struct {
Expand Down Expand Up @@ -223,18 +224,18 @@ func benchmarkArrayGet(b *testing.B, initialArraySize, numberOfOps int) {

array := setupArray(b, r, storage, initialArraySize)

var storable Storable
var value Value

b.StartTimer()

for i := 0; i < b.N; i++ {
for i := 0; i < numberOfOps; i++ {
index := r.Intn(int(array.Count()))
storable, _ = array.Get(uint64(index))
value, _ = array.Get(uint64(index))
}
}

noop = storable
noopValue = value
}

func benchmarkArrayInsert(b *testing.B, initialArraySize, numberOfOps int) {
Expand Down Expand Up @@ -307,7 +308,7 @@ func benchmarkArrayRemoveAll(b *testing.B, initialArraySize int) {
}
}

noop = storable
noopStorable = storable
}

func benchmarkArrayPopIterate(b *testing.B, initialArraySize int) {
Expand Down Expand Up @@ -336,7 +337,7 @@ func benchmarkArrayPopIterate(b *testing.B, initialArraySize int) {
}
}

noop = storable
noopStorable = storable
}

func benchmarkNewArrayFromAppend(b *testing.B, initialArraySize int) {
Expand Down
9 changes: 3 additions & 6 deletions array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ func verifyArray(

// Verify array elements
for i, v := range values {
s, err := array.Get(uint64(i))
require.NoError(t, err)

e, err := s.StoredValue(array.Storage)
e, err := array.Get(uint64(i))
require.NoError(t, err)

valueEqual(t, typeInfoComparator, v, e)
Expand Down Expand Up @@ -153,8 +150,8 @@ func TestArrayAppendAndGet(t *testing.T) {
require.NoError(t, err)
}

storable, err := array.Get(array.Count())
require.Nil(t, storable)
e, err := array.Get(array.Count())
require.Nil(t, e)
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
Expand Down
6 changes: 1 addition & 5 deletions cmd/stress/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,14 +417,10 @@ func checkArrayDataLoss(array *atree.Array, values []atree.Value) error {

// Check every element
for i, v := range values {
storable, err := array.Get(uint64(i))
convertedValue, err := array.Get(uint64(i))
if err != nil {
return fmt.Errorf("failed to get element at %d: %w", i, err)
}
convertedValue, err := storable.StoredValue(array.Storage)
if err != nil {
return fmt.Errorf("failed to convert storable to value at %d: %w", i, err)
}
err = valueEqual(v, convertedValue)
if err != nil {
return fmt.Errorf("failed to compare %s and %s: %w", v, convertedValue, err)
Expand Down

0 comments on commit 0c73dea

Please sign in to comment.