Skip to content

Commit

Permalink
Change Array.Get() to return Value instead of Storable
Browse files Browse the repository at this point in the history
This commit changes Array.Get() to return Value instead of Storable.

Currently, Array.Get() returns Storable and client converts
returned Storable to Value.  However, it only makes sense to
return Storable if client needs to remove register (slab) by
StorageID (StorageIDStorable).

Get() should only provide value without possibility of client
manipulating the underlying storable.
  • Loading branch information
fxamacker committed Jun 27, 2023
1 parent 8db67b9 commit 044db01
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 044db01

Please sign in to comment.