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.

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

Sync stable-cadence branch with master #2455

Merged
merged 201 commits into from
Apr 26, 2023

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Apr 20, 2023

Description

Sync stable-cadence branch with master


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

fxamacker and others added 30 commits February 28, 2023 12:05
This is to allow CCF codec to use new StreamEncoder.Close()
function provided by latest fxamacker/cbor.
- added NewMeteredUFix64FromUint64 to create metered UFix64 from uint64.
- added NewMeteredFix64FromInt64 to create metered Fix64 from int64.
Cadence Compact Format (CCF) is a data format designed for compact,
efficient, and deterministic encoding of Cadence external values.

CCF obsoletes JSON-Cadence Data Interchange Format (JCDIF) for
use cases that do not require JSON.

Unlike JCDIF, CCF Specifications explicitly defines requirements for
- well-formed encodings
- valid encodings
- deterministic encodings

CCF is hybrid data format. CCF-based messages can be:
- fully self-describing or
- partially self-describing.

Both CCF modes are more compact than JSON-based messages. CCF-based
protocols can send Cadence metadata just once for all messages of
that type. Malformed data can be detected without Cadence metadata
and without creating Cadence objects.

CCF codec implements CCF Specification (RC1), which is
temporarily at github.com/fxamacker/ccf_draft. The CCF specs
will be hosted under github.com/onflow after it is updated,
cleaned up, and reformatted.
CCF obsoletes JSON-Cadence Data Interchange Format for use cases
that do not require JSON.  Given this, preliminary comparisons are
described here for the CCF codec implementing CCF Specifications (RC1).

PRELIMINARY COMPARISONS

Comparisons used 48,309 events from a single mainnet transaction.

There were 9 event types. To simplify benchmark code, the first event's
value in each of the 9 event types was used.

CCF's partially self-describing mode (aka "detached" mode) would be
even smaller than this (e.g. maybe less than 1/4 the size of JSON when
Flow eventually supports detached mode).

SIZE COMPARISON

Encoding | Num Events | Encoded size | Comments
-------- | ---------- | ------------ | --------
JSON     |     48,309 |   13,858,836 | JSON-Cadence Data Interchange
CCF      |     48,309 |    6,159,931 | CCF in fully self-describing mode

CCF SPEED AND MEMORY COMPARISONS

This isn't apples to apples comparison.   JSON data isn't sorted, etc.
- CCF encoder sorts data to encode event data deterministically.
- CCF decoder also verifies event data is sorted, well-formed, and valid.
- CCF encoding is less than 1/2 size of JSON-Cadence Data Interchange.

ENCODER COMPARISON

48k_events_encode_json.log │  48k_events_encode_ccf.log
          sec/op           │   sec/op       vs base
       89.84m ± 17%            69.28m ± 3%  -22.88%

48k_events_encode_json.log │  48k_events_encode_ccf.log
            B/op           │     B/op        vs base
       32.45Mi ± 0%           25.82Mi ± 0%  -20.45%

48k_events_encode_json.log │  48k_events_encode_ccf.log
         allocs/op         │  allocs/op     vs base
        756.6k ± 0%            370.4k ± 0%  -51.05%

DECODER COMPARISON

48k_events_decode_json.log │   48k_events_decode_ccf.log
          sec/op           │   sec/op       vs base
        646.2m ± 8%            158.3m ± 5%  -75.50%

48k_events_decode_json.log │   48k_events_decode_ccf.log
           B/op            │     B/op        vs base
      234.97Mi ± 0%            56.16Mi ± 0%  -76.10%

48k_events_decode_json.log │   48k_events_decode_ccf.log
        allocs/op          │  allocs/op     vs base
        4.746M ± 0%            1.288M ± 0%  -72.86%

Benchmarked using Go 1.19.6, linux_amd64, i5-13600k.  Results are
subject to change because CCF codec reference implementation in Go
has not yet been reviewed or merged into onflow/cadence yet.
Added comment stating that cadence.String and cadence.Character
must be valid UTF-8 and it is the application's responsibility
to provide the CCF encoder with valid UTF-8 strings.

"Valid CCF Encoding Requirements" in CCF Specification states:

    "Encoders are not required to check for invalid input items
    (e.g. invalid UTF-8 strings, duplicate dictionary keys, etc.)
    Applications MUST NOT provide invalid items to encoders."
Updated Go comments containing CCF specs in CDDL notation. Specifically,
changed CDDL notation representing "one or more" to "zero or more" for:
- composite-value.fields
- composite-type.fields
- composite-type-value.fields

See CCF specification PRs:
- fxamacker/ccf_draft#66
- fxamacker/ccf_draft#67
Updated Go comments containing CCF specs in CDDL notation. Specifically,
changed CDDL notation representing "one or more" to "zero or more" for:
- restricted-type.restrictions
- restricted-type-value.restrictions

See CCF specification PRs:
- fxamacker/ccf_draft#66
- fxamacker/ccf_draft#67
CCF specs were updated to allow "zero or more" sortable items
rather than "one or more".

Given this, update CCF codec to check for zero items before sorting.

Relevant changes to CCF specification include:
- fxamacker/ccf_draft#66
- fxamacker/ccf_draft#67

Co-authored-by: Bastian Müller <[email protected]>
- Return Cadence UnexpectedError for implemention errors.
- Rethrow Go runtime errors and Cadence internal errors in:
  - CCF Encoder.Encode()
  - CCF Decoder.Decode()
Explained why tids isn't passed down and other related aspects.
@SupunS SupunS changed the base branch from master to feature/stable-cadence April 20, 2023 00:09
@github-actions
Copy link

github-actions bot commented Apr 20, 2023

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/stable-cadence commit be303b7
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

old.txtnew.txt
time/opdelta
CheckContractInterfaceFungibleTokenConformance-2114µs ± 0%114µs ± 0%~(p=1.000 n=1+1)
ContractInterfaceFungibleToken-239.3µs ± 0%39.1µs ± 0%~(p=1.000 n=1+1)
ExportType/composite_type-2334ns ± 0%326ns ± 0%~(p=1.000 n=1+1)
ExportType/simple_type-252.6ns ± 0%52.2ns ± 0%~(p=1.000 n=1+1)
InterpretRecursionFib-22.48ms ± 0%2.54ms ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_interpreter-21.08µs ± 0%1.12µs ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_sub-interpreter-2572ns ± 0%612ns ± 0%~(p=1.000 n=1+1)
ParseArray-27.60ms ± 0%7.57ms ± 0%~(p=1.000 n=1+1)
ParseDeploy/byte_array-211.5ms ± 0%11.6ms ± 0%~(p=1.000 n=1+1)
ParseDeploy/decode_hex-21.24ms ± 0%1.20ms ± 0%~(p=1.000 n=1+1)
ParseFungibleToken/With_memory_metering-2184µs ± 0%187µs ± 0%~(p=1.000 n=1+1)
ParseFungibleToken/Without_memory_metering-2145µs ± 0%146µs ± 0%~(p=1.000 n=1+1)
ParseInfix-27.00µs ± 0%6.99µs ± 0%~(p=1.000 n=1+1)
QualifiedIdentifierCreation/One_level-22.41ns ± 0%2.41ns ± 0%~(p=1.000 n=1+1)
QualifiedIdentifierCreation/Three_levels-2121ns ± 0%116ns ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-25.06ms ± 0%5.15ms ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-27.58µs ± 0%4.18µs ± 0%~(p=1.000 n=1+1)
SuperTypeInference/arrays-2305ns ± 0%297ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/composites-2130ns ± 0%132ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/integers-296.8ns ± 0%92.6ns ± 0%~(p=1.000 n=1+1)
ValueIsSubtypeOfSemaType-290.0ns ± 0%92.1ns ± 0%~(p=1.000 n=1+1)
 
alloc/opdelta
CheckContractInterfaceFungibleTokenConformance-250.3kB ± 0%50.7kB ± 0%~(p=1.000 n=1+1)
ContractInterfaceFungibleToken-223.9kB ± 0%24.1kB ± 0%~(p=1.000 n=1+1)
ExportType/composite_type-2136B ± 0%136B ± 0%~(all equal)
ExportType/simple_type-20.00B 0.00B ~(all equal)
InterpretRecursionFib-21.00MB ± 0%1.00MB ± 0%~(all equal)
NewInterpreter/new_interpreter-2768B ± 0%768B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-2200B ± 0%200B ± 0%~(all equal)
ParseArray-22.72MB ± 0%2.72MB ± 0%~(p=1.000 n=1+1)
ParseDeploy/byte_array-24.09MB ± 0%4.26MB ± 0%~(p=1.000 n=1+1)
ParseDeploy/decode_hex-2214kB ± 0%214kB ± 0%~(p=1.000 n=1+1)
ParseFungibleToken/With_memory_metering-229.3kB ± 0%29.3kB ± 0%~(all equal)
ParseFungibleToken/Without_memory_metering-229.3kB ± 0%29.3kB ± 0%~(p=1.000 n=1+1)
ParseInfix-21.92kB ± 0%1.92kB ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-264.0B ± 0%64.0B ± 0%~(all equal)
RuntimeResourceDictionaryValues-22.28MB ± 0%2.28MB ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-22.86kB ± 0%2.86kB ± 0%~(all equal)
SuperTypeInference/arrays-296.0B ± 0%96.0B ± 0%~(all equal)
SuperTypeInference/composites-20.00B 0.00B ~(all equal)
SuperTypeInference/integers-20.00B 0.00B ~(all equal)
ValueIsSubtypeOfSemaType-248.0B ± 0%48.0B ± 0%~(all equal)
 
allocs/opdelta
CheckContractInterfaceFungibleTokenConformance-2811 ± 0%811 ± 0%~(all equal)
ContractInterfaceFungibleToken-2371 ± 0%371 ± 0%~(all equal)
ExportType/composite_type-23.00 ± 0%3.00 ± 0%~(all equal)
ExportType/simple_type-20.00 0.00 ~(all equal)
InterpretRecursionFib-218.9k ± 0%18.9k ± 0%~(all equal)
NewInterpreter/new_interpreter-213.0 ± 0%13.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-24.00 ± 0%4.00 ± 0%~(all equal)
ParseArray-259.6k ± 0%59.6k ± 0%~(all equal)
ParseDeploy/byte_array-289.4k ± 0%89.4k ± 0%~(all equal)
ParseDeploy/decode_hex-263.0 ± 0%63.0 ± 0%~(all equal)
ParseFungibleToken/With_memory_metering-2778 ± 0%778 ± 0%~(all equal)
ParseFungibleToken/Without_memory_metering-2778 ± 0%778 ± 0%~(all equal)
ParseInfix-248.0 ± 0%48.0 ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-22.00 ± 0%2.00 ± 0%~(all equal)
RuntimeResourceDictionaryValues-236.9k ± 0%36.9k ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-244.0 ± 0%44.0 ± 0%~(all equal)
SuperTypeInference/arrays-23.00 ± 0%3.00 ± 0%~(all equal)
SuperTypeInference/composites-20.00 0.00 ~(all equal)
SuperTypeInference/integers-20.00 0.00 ~(all equal)
ValueIsSubtypeOfSemaType-21.00 ± 0%1.00 ± 0%~(all equal)
 

@SupunS SupunS force-pushed the supun/sync-stable-cadence branch 2 times, most recently from 6e89c5d to adca90b Compare April 20, 2023 02:16
@SupunS
Copy link
Member Author

SupunS commented Apr 20, 2023

There were quite a few conflicts, mostly between the #2401, and the purity changes.

In addition to conflicts, had to do the following behavioral changes:

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Merging #2455 (8919359) into feature/stable-cadence (be303b7) will decrease coverage by 0.28%.
The diff coverage is 79.63%.

@@                    Coverage Diff                     @@
##           feature/stable-cadence    #2455      +/-   ##
==========================================================
- Coverage                   78.75%   78.48%   -0.28%     
==========================================================
  Files                         317      326       +9     
  Lines                       69296    72949    +3653     
==========================================================
+ Hits                        54574    57253    +2679     
- Misses                      12919    13615     +696     
- Partials                     1803     2081     +278     
Flag Coverage Δ
unittests 78.48% <79.63%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
runtime/common/memorykind_string.go 40.00% <ø> (ø)
runtime/common/metering.go 92.32% <ø> (ø)
runtime/parser/parser.go 90.95% <0.00%> (-1.55%) ⬇️
runtime/pretty/print.go 75.45% <ø> (ø)
runtime/repl.go 0.00% <0.00%> (ø)
runtime/sema/resources.go 88.44% <0.00%> (ø)
runtime/stdlib/flow.go 86.66% <ø> (ø)
types.go 85.88% <ø> (+3.32%) ⬆️
values.go 75.60% <ø> (+0.20%) ⬆️
runtime/ast/expression.go 92.49% <50.00%> (ø)
... and 43 more

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@SupunS SupunS marked this pull request as ready for review April 21, 2023 14:52
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work resolving all those conflicts! 💪

encoding/ccf/encode.go Outdated Show resolved Hide resolved
runtime/sema/gen/main.go Show resolved Hide resolved
docs/language/accounts.mdx Outdated Show resolved Hide resolved
@dsainati1
Copy link
Contributor

Would like to get this merged soon since I'm going to be reworking the entitlement stack.

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@SupunS SupunS merged commit 0259a36 into feature/stable-cadence Apr 26, 2023
@SupunS SupunS deleted the supun/sync-stable-cadence branch April 26, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants