Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use atree readonly iterators when mutation of values is not needed #3170

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Use atree readonly iterators when mutation of values is not needed #3170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f70345
Use atree readonly iterators when feasible
fxamacker Mar 12, 2024
b68900d
Use atree non-readonly iterators in some places
fxamacker Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions runtime/interpreter/interpreter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2061,7 +2061,7 @@ func (interpreter *Interpreter) convert(value Value, valueType, targetType sema.

array := arrayValue.array

iterator, err := array.Iterator()
iterator, err := array.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down Expand Up @@ -2102,10 +2102,7 @@ func (interpreter *Interpreter) convert(value Value, valueType, targetType sema.

dictionary := dictValue.dictionary

valueComparator := newValueComparator(interpreter, locationRange)
hashInputProvider := newHashInputProvider(interpreter, locationRange)

iterator, err := dictionary.Iterator(valueComparator, hashInputProvider)
iterator, err := dictionary.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down
29 changes: 20 additions & 9 deletions runtime/interpreter/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1973,12 +1973,14 @@ func (v *ArrayValue) Concat(interpreter *Interpreter, locationRange LocationRang

first := true

firstIterator, err := v.array.Iterator()
// Use ReadOnlyIterator here because new ArrayValue is created with elements copied (not removed) from original value.
firstIterator, err := v.array.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}

secondIterator, err := other.array.Iterator()
// Use ReadOnlyIterator here because new ArrayValue is created with elements copied (not removed) from original value.
secondIterator, err := other.array.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down Expand Up @@ -3075,7 +3077,8 @@ func (v *ArrayValue) Slice(
})
}

iterator, err := v.array.RangeIterator(uint64(fromIndex), uint64(toIndex))
// Use ReadOnlyIterator here because new ArrayValue is created from elements copied (not removed) from original ArrayValue.
iterator, err := v.array.ReadOnlyRangeIterator(uint64(fromIndex), uint64(toIndex))
if err != nil {

var sliceOutOfBoundsError *atree.SliceOutOfBoundsError
Expand Down Expand Up @@ -3192,6 +3195,7 @@ func (v *ArrayValue) Filter(
return invocation
}

// TODO: Use ReadOnlyIterator here if procedure doesn't change array elements.
iterator, err := v.array.Iterator()
if err != nil {
panic(errors.NewExternalError(err))
Expand Down Expand Up @@ -3293,6 +3297,7 @@ func (v *ArrayValue) Map(
panic(errors.NewUnreachableError())
}

// TODO: Use ReadOnlyIterator here if procedure doesn't change map values.
iterator, err := v.array.Iterator()
if err != nil {
panic(errors.NewExternalError(err))
Expand Down Expand Up @@ -3357,7 +3362,8 @@ func (v *ArrayValue) ToVariableSized(
panic(errors.NewUnreachableError())
}

iterator, err := v.array.Iterator()
// Use ReadOnlyIterator here because ArrayValue elements are copied (not removed) from original ArrayValue.
iterator, err := v.array.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down Expand Up @@ -3417,7 +3423,8 @@ func (v *ArrayValue) ToConstantSized(
panic(errors.NewUnreachableError())
}

iterator, err := v.array.Iterator()
// Use ReadOnlyIterator here because ArrayValue elements are copied (not removed) from original ArrayValue.
iterator, err := v.array.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down Expand Up @@ -17936,6 +17943,9 @@ func (v *CompositeValue) ForEachFieldName(
f func(fieldName string) (resume bool),
) {
iterate := func(fn atree.MapElementIterationFunc) error {
// Use NonReadOnlyIterator because we are not sure if it's guaranteed that
// all uses of CompositeValue.ForEachFieldName are only read-only.
// TODO: determine if all uses of CompositeValue.ForEachFieldName are read-only.
return v.dictionary.IterateKeys(
StringAtreeValueComparator,
StringAtreeValueHashInput,
Expand Down Expand Up @@ -18636,6 +18646,9 @@ func (v *DictionaryValue) IterateKeys(
valueComparator := newValueComparator(interpreter, locationRange)
hashInputProvider := newHashInputProvider(interpreter, locationRange)
iterate := func(fn atree.MapElementIterationFunc) error {
// Use NonReadOnlyIterator because we are not sure if f in
// all uses of DictionaryValue.IterateKeys are always read-only.
// TODO: determine if all uses of f are read-only.
return v.dictionary.IterateKeys(
valueComparator,
hashInputProvider,
Expand Down Expand Up @@ -19110,10 +19123,8 @@ func (v *DictionaryValue) GetMember(

case "values":

valueComparator := newValueComparator(interpreter, locationRange)
hashInputProvider := newHashInputProvider(interpreter, locationRange)

iterator, err := v.dictionary.Iterator(valueComparator, hashInputProvider)
// Use ReadOnlyIterator here because new ArrayValue is created with copied elements (not removed) from original.
iterator, err := v.dictionary.ReadOnlyIterator()
if err != nil {
panic(errors.NewExternalError(err))
}
Expand Down