Conversation
This commit optimizes copying arrays or maps that don't
require deep copying or slab (aka register) ops by using Atree's
new Array.Copy() and OrderedMap.Copy().
Currently, ArrayValue, DictionaryValue, and CompositeValue
use these Atree functions to copy arrays/maps in Transfer():
- atree.NewArrayFromBatchData()
- atree.NewMapFromBatchData()
With this commit, Cadence Transfer() uses Atree's Copy() if:
- The array or map is small (a single slab container).
- All elements are not containers or references to other slabs.
- All elements don't wrap containers or references to other slabs.
- All elements return true from Storable.CanCopy().
Otherwise, Transfer() continues to use NewArrayFromBatchData() or
NewMapFromBatchData() for arrays or maps that require deep copy or
slab operations.
Benchmarks to transfer Cadence [UInt8]:
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
ByteArrayTransfer-12 1528.5n ± 2% 755.6n ± 2% -50.57% (p=0.000 n=20)
│ before.txt │ after.txt │
│ B/op │ B/op vs base │
ByteArrayTransfer-12 1551.5 ± 0% 1014.0 ± 0% -34.64% (p=0.000 n=20)
│ before.txt │ after.txt │
│ allocs/op │ allocs/op vs base │
ByteArrayTransfer-12 14.000 ± 0% 7.000 ± 0% -50.00% (p=0.000 n=20)
Benchmarks to transfer Cadence enum:
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
EMVAddressTransfer-12 2.692µ ± 1% 2.114µ ± 1% -21.49% (p=0.000 n=20)
│ before.txt │ after.txt │
│ B/op │ B/op vs base │
EMVAddressTransfer-12 3.163Ki ± 0% 2.407Ki ± 0% -23.90% (p=0.000 n=20)
│ before.txt │ after.txt │
│ allocs/op │ allocs/op vs base │
EMVAddressTransfer-12 36.00 ± 0% 29.00 ± 0% -19.44% (p=0.000 n=20)
Benchmarks to transfer EVMAddress (composite value with [UInt8: 20] field):
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
EnumTransfer-12 1228.0n ± 1% 815.6n ± 3% -33.59% (p=0.000 n=20)
│ before.txt │ after.txt │
│ B/op │ B/op vs base │
EnumTransfer-12 1719.0 ± 0% 843.5 ± 1% -50.93% (p=0.000 n=20)
│ before.txt │ after.txt │
│ allocs/op │ allocs/op vs base │
EnumTransfer-12 19.00 ± 0% 13.00 ± 0% -31.58% (p=0.000 n=20)
fxamacker
force-pushed
the
fxamacker/optimize-cadence-value-transfer
branch
from
3820385 to
a6b78c4
Compare
fxamacker
commented
Mar 4, 2026
fxamacker
commented
Mar 4, 2026
fxamacker
commented
Mar 4, 2026
turbolent
reviewed
Mar 4, 2026
Member
There was a problem hiding this comment.
Great work! Great idea to optimize this common case with a "fast path" 👏
The changes in the Transfer methods look good. Just a few minor TODOs / concerns:
- Improve method naming (see atree PR)
- Resolve question re: large values with separate slab
- Add back computation metering
- Add back tracing
This refactor allows fewer types to qualify for skipping calling `CanCopyNonRefSimple` for all elements in a container.
…into fxamacker/optimize-cadence-value-transfer
turbolent
reviewed
Mar 5, 2026
turbolent
reviewed
Mar 5, 2026
…tree arrays and maps
While at it, also added more comments.
Member
Author
|
@turbolent BTW, I spotted this while adding more comments. It seems the raw value for enums is an integer subtype, which can be So I removed the fast path for enum values. |
Member
|
@fxamacker Good catch! Would it be possible to add a test case for an enum with a very large integer that will trigger it to be stored in a separate slab? Such a regression test would help us accidentally re-add the fast path / optimization |
Base automatically changed from
fxamacker/optimize-array-and-go-byte-slice-conversion
to
master
March 9, 2026 16:59
An enum value can be of any integer subtype, and very large integer values are not copyable since they are stored in their own slabs. This commit adds a unit test to test computation metering when transferring a very large enum value with a raw value stored in a separate slab.
Member
|
Triggering stuck CI by closing and reopening |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Benchstat comparison
Results
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
@turbolent Good idea! I pushed commit f685701 to add tests for enum value transfers for both small and large (over 500 bytes) enum values. |
Member
Member
Author
|
Thanks @turbolent for adding new metering kinds. Outside this PR, @janezpodhostnik will need to assign weights to the new metering kinds. 🙏 |
This commit adds a new internal error StorableCopyError, which is returned from Storable.CopyNonRefSimple() if the given storable type can't be copied.
12 tasks
CopyNonRefSimple() is an optimized path for Transfer(), and both approaches should return the same value. For example, if Transfer() returns a shallow copy, CopyNonRefSimple() should also return a shallow copy. This commit updates CopyNonRefSimple() to return the same value as Transfer(). This commit also adds comments in both functions to mention that returned values from these two functions should be kept in sync.
SupunS
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates onflow/flow-go#8447 onflow/flow-go#8401
Requires onflow/atree#633
This optimization benefits EVM, FVM, etc. For example, an open yield vault transaction indirectly uses
Transfer()more than 2900 times. More than 1800 of those 2900Transfer()are optimized by this.This PR optimizes copying arrays or maps that don't require deep copying or slab (aka payload) operations by using Atree's new
Array.Copy()andOrderedMap.Copy(). For example, copying byte array is 2x faster and uses half the memory allocations.EVMAddressand other frequently used data like hashes currently use byte arrays, so the 2x speedup and reduced memory use for copying them can be worthwhile. Other data such as Cadence enum is about 1.5x faster to copy.Currently,
ArrayValue,DictionaryValue, andCompositeValueuse these Atree functions to copy arrays/maps inTransfer():atree.NewArrayFromBatchData()atree.NewMapFromBatchData()With this PR, Cadence
Transfer()uses Atree'sCopy()if:Storable.CanCopy().Otherwise,
Transfer()continues to useNewArrayFromBatchData()orNewMapFromBatchData()for arrays or maps that require deep copy or slab operations.NOTE: To be conservative, some
Storabletypes use the old way of copying but it might be OK to use Atree'sCopy()for some of them too if the Cadence team determines it would be safe. See "Caveats" section below for more details.Benchmarks
Benchmarks to transfer Cadence [UInt8]:
Benchmarks to transfer Cadence enum:
Benchmarks to transfer EVMAddress (composite value with [UInt8: 20] field):
Caveats
To be conservative,
CanCopy()returnsfalsefor theseStorabletypes, and they continue to use the old way of copying:NonStorableAccountCapabilityControllerValuePublishedValueStorageCapabilityControllerValueIf needed, the Cadence team can look into these 4 types to determine if some of them are safe to copy their values using the new Atree
Copy()function.TODO
masterbranchFiles changedin the Github PR explorer