Skip to content

Commit

Permalink
Merge pull request #327 from onflow/fxamacker/refactor-haspointer-for…
Browse files Browse the repository at this point in the history
…-register-inlining

Refactor `hasPointer` for register inlining
  • Loading branch information
fxamacker authored Aug 2, 2023
2 parents 24f8172 + c2f6044 commit 402ea64
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 72 deletions.
2 changes: 1 addition & 1 deletion array.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func (a *ArrayDataSlab) Encode(enc *Encoder) error {

func (a *ArrayDataSlab) hasPointer() bool {
for _, e := range a.elements {
if _, ok := e.(SlabIDStorable); ok {
if hasPointer(e) {
return true
}
}
Expand Down
74 changes: 21 additions & 53 deletions map.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ type element interface {

Encode(*Encoder) error

HasPointer() bool
hasPointer() bool

Size() uint32

Expand Down Expand Up @@ -166,7 +166,7 @@ type elements interface {

Encode(*Encoder) error

HasPointer() bool
hasPointer() bool

firstKey() Digest

Expand All @@ -178,11 +178,9 @@ type elements interface {
}

type singleElement struct {
key MapKey
value MapValue
size uint32
keyPointer bool
valuePointer bool
key MapKey
value MapValue
size uint32
}

var _ element = &singleElement{}
Expand Down Expand Up @@ -464,22 +462,10 @@ func newSingleElement(storage SlabStorage, address Address, key Value, value Val
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get value's storable")
}

var keyPointer bool
if _, ok := ks.(SlabIDStorable); ok {
keyPointer = true
}

var valuePointer bool
if _, ok := vs.(SlabIDStorable); ok {
valuePointer = true
}

return &singleElement{
key: ks,
value: vs,
size: singleElementPrefixSize + ks.ByteSize() + vs.ByteSize(),
keyPointer: keyPointer,
valuePointer: valuePointer,
key: ks,
value: vs,
size: singleElementPrefixSize + ks.ByteSize() + vs.ByteSize(),
}, nil
}

Expand All @@ -505,22 +491,10 @@ func newSingleElementFromData(cborDec *cbor.StreamDecoder, decodeStorable Storab
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to decode value's storable")
}

var keyPointer bool
if _, ok := key.(SlabIDStorable); ok {
keyPointer = true
}

var valuePointer bool
if _, ok := value.(SlabIDStorable); ok {
valuePointer = true
}

return &singleElement{
key: key,
value: value,
size: singleElementPrefixSize + key.ByteSize() + value.ByteSize(),
keyPointer: keyPointer,
valuePointer: valuePointer,
key: key,
value: value,
size: singleElementPrefixSize + key.ByteSize() + value.ByteSize(),
}, nil
}

Expand Down Expand Up @@ -603,14 +577,8 @@ func (e *singleElement) Set(
return nil, nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get value's storable")
}

valuePointer := false
if _, ok := valueStorable.(SlabIDStorable); ok {
valuePointer = true
}

e.value = valueStorable
e.size = singleElementPrefixSize + e.key.ByteSize() + e.value.ByteSize()
e.valuePointer = valuePointer
return e, existingValue, nil
}

Expand Down Expand Up @@ -676,8 +644,8 @@ func (e *singleElement) Remove(storage SlabStorage, _ Digester, _ uint, _ Digest
return nil, nil, nil, NewKeyNotFoundError(key)
}

func (e *singleElement) HasPointer() bool {
return e.keyPointer || e.valuePointer
func (e *singleElement) hasPointer() bool {
return hasPointer(e.key) || hasPointer(e.value)
}

func (e *singleElement) Size() uint32 {
Expand Down Expand Up @@ -849,8 +817,8 @@ func (e *inlineCollisionGroup) Remove(storage SlabStorage, digester Digester, le
return k, v, e, nil
}

func (e *inlineCollisionGroup) HasPointer() bool {
return e.elements.HasPointer()
func (e *inlineCollisionGroup) hasPointer() bool {
return e.elements.hasPointer()
}

func (e *inlineCollisionGroup) Size() uint32 {
Expand Down Expand Up @@ -1019,7 +987,7 @@ func (e *externalCollisionGroup) Remove(storage SlabStorage, digester Digester,
return k, v, e, nil
}

func (e *externalCollisionGroup) HasPointer() bool {
func (e *externalCollisionGroup) hasPointer() bool {
return true
}

Expand Down Expand Up @@ -1530,9 +1498,9 @@ func (e *hkeyElements) Element(i int) (element, error) {
return e.elems[i], nil
}

func (e *hkeyElements) HasPointer() bool {
func (e *hkeyElements) hasPointer() bool {
for _, elem := range e.elems {
if elem.HasPointer() {
if elem.hasPointer() {
return true
}
}
Expand Down Expand Up @@ -2015,9 +1983,9 @@ func (e *singleElements) CanLendToRight(_ uint32) bool {
return false
}

func (e *singleElements) HasPointer() bool {
func (e *singleElements) hasPointer() bool {
for _, elem := range e.elems {
if elem.HasPointer() {
if elem.hasPointer() {
return true
}
}
Expand Down Expand Up @@ -2255,7 +2223,7 @@ func (m *MapDataSlab) Encode(enc *Encoder) error {
}

func (m *MapDataSlab) hasPointer() bool {
return m.elements.HasPointer()
return m.elements.hasPointer()
}

func (m *MapDataSlab) ChildStorables() []Storable {
Expand Down
18 changes: 0 additions & 18 deletions map_debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,11 +726,6 @@ func validSingleElement(
err error,
) {

// Verify key pointer
if _, keyPointer := e.key.(SlabIDStorable); e.keyPointer != keyPointer {
return 0, 0, NewFatalError(fmt.Errorf("element %s keyPointer %t is wrong, want %t", e, e.keyPointer, keyPointer))
}

// Verify key
kv, err := e.key.StoredValue(storage)
if err != nil {
Expand All @@ -744,11 +739,6 @@ func validSingleElement(
return 0, 0, fmt.Errorf("element %s key isn't valid: %w", e, err)
}

// Verify value pointer
if _, valuePointer := e.value.(SlabIDStorable); e.valuePointer != valuePointer {
return 0, 0, NewFatalError(fmt.Errorf("element %s valuePointer %t is wrong, want %t", e, e.valuePointer, valuePointer))
}

// Verify value
vv, err := e.value.StoredValue(storage)
if err != nil {
Expand Down Expand Up @@ -1272,14 +1262,6 @@ func mapSingleElementEqual(
return NewFatalError(fmt.Errorf("singleElement size %d is wrong, want %d", actual.size, expected.size))
}

if expected.keyPointer != actual.keyPointer {
return NewFatalError(fmt.Errorf("singleElement keyPointer %t is wrong, want %t", actual.keyPointer, expected.keyPointer))
}

if expected.valuePointer != actual.valuePointer {
return NewFatalError(fmt.Errorf("singleElement valuePointer %t is wrong, want %t", actual.valuePointer, expected.valuePointer))
}

if !compare(expected.key, actual.key) {
return NewFatalError(fmt.Errorf("singleElement key %v is wrong, want %v", actual.key, expected.key))
}
Expand Down
16 changes: 16 additions & 0 deletions storable.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ type Storable interface {
ChildStorables() []Storable
}

type containerStorable interface {
Storable
hasPointer() bool
}

func hasPointer(storable Storable) bool {
if cs, ok := storable.(containerStorable); ok {
return cs.hasPointer()
}
return false
}

const (
CBORTagInlineCollisionGroup = 253
CBORTagExternalCollisionGroup = 254
Expand All @@ -48,6 +60,10 @@ type SlabIDStorable SlabID

var _ Storable = SlabIDStorable{}

func (v SlabIDStorable) hasPointer() bool {
return true
}

func (v SlabIDStorable) ChildStorables() []Storable {
return nil
}
Expand Down
7 changes: 7 additions & 0 deletions storable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,13 @@ type SomeStorable struct {

var _ Storable = SomeStorable{}

func (v SomeStorable) hasPointer() bool {
if ms, ok := v.Storable.(containerStorable); ok {
return ms.hasPointer()
}
return false
}

func (v SomeStorable) ByteSize() uint32 {
// tag number (2 bytes) + encoded content
return 2 + v.Storable.ByteSize()
Expand Down

0 comments on commit 402ea64

Please sign in to comment.