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

Flowkit version update #1534

Merged
merged 11 commits into from
Apr 13, 2022
Merged

Flowkit version update #1534

merged 11 commits into from
Apr 13, 2022

Conversation

devbugging
Copy link
Contributor

@devbugging devbugging commented Mar 25, 2022

Description

The flowkit version was updated and API changes were implemented.


  • 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

@devbugging
Copy link
Contributor Author

Semgrep is failing, I'm assuming there is no snapshot file in the LS module? should that be initialized?

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2022

Codecov Report

Merging #1534 (23f10b4) into master (2aaae59) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1534   +/-   ##
=======================================
  Coverage   74.64%   74.64%           
=======================================
  Files         288      288           
  Lines       55364    55364           
=======================================
+ Hits        41327    41329    +2     
+ Misses      12538    12536    -2     
  Partials     1499     1499           
Flag Coverage Δ
unittests 74.64% <ø> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
runtime/interpreter/storage.go 72.78% <0.00%> (+1.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2aaae59...23f10b4. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Mar 25, 2022

Cadence Benchstat comparison

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

Results

old.txtnew.txt
time/opdelta
NewInterpreter/new_interpreter-21.43µs ± 3%1.46µs ± 5%+2.37%(p=0.049 n=7+7)
RuntimeResourceDictionaryValues-28.07ms ± 4%7.99ms ± 6%~(p=0.710 n=7+7)
RuntimeFungibleTokenTransfer-21.51ms ±20%1.66ms ±22%~(p=0.209 n=7+7)
Transfer-2107ns ± 2%108ns ± 2%~(p=0.066 n=7+7)
ParseArray-215.4ms ± 2%15.6ms ± 2%~(p=0.589 n=6+6)
ParseDeploy/byte_array-224.5ms ± 4%24.5ms ± 4%~(p=0.902 n=7+7)
ParseDeploy/decode_hex-21.53ms ± 3%1.53ms ± 2%~(p=0.805 n=7+7)
ParseInfix-210.4µs ± 2%10.6µs ± 2%~(p=0.138 n=6+7)
ParseFungibleToken-2237µs ± 5%236µs ± 2%~(p=0.805 n=7+7)
QualifiedIdentifierCreation/One_level-23.50ns ± 1%3.50ns ± 1%~(p=1.000 n=6+6)
QualifiedIdentifierCreation/Three_levels-2181ns ± 2%182ns ± 4%~(p=0.456 n=7+7)
ContractInterfaceFungibleToken-251.0µs ± 4%50.1µs ± 2%~(p=0.259 n=7+7)
CheckContractInterfaceFungibleTokenConformance-2182µs ± 2%184µs ± 5%~(p=0.710 n=7+7)
NewInterpreter/new_sub-interpreter-22.81µs ± 4%2.78µs ± 2%~(p=0.165 n=7+7)
InterpretRecursionFib-23.29ms ± 2%3.29ms ± 4%~(p=0.902 n=7+7)
 
alloc/opdelta
RuntimeResourceDictionaryValues-22.25MB ± 0%2.25MB ± 0%~(p=0.383 n=7+7)
RuntimeFungibleTokenTransfer-2273kB ± 0%273kB ± 0%~(p=0.477 n=7+7)
Transfer-248.0B ± 0%48.0B ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-264.0B ± 0%64.0B ± 0%~(all equal)
ContractInterfaceFungibleToken-226.6kB ± 0%26.6kB ± 0%~(p=0.592 n=7+7)
CheckContractInterfaceFungibleTokenConformance-266.2kB ± 0%66.2kB ± 0%~(all equal)
NewInterpreter/new_interpreter-2848B ± 0%848B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-21.34kB ± 0%1.34kB ± 0%~(all equal)
InterpretRecursionFib-21.14MB ± 0%1.14MB ± 0%~(all equal)
 
allocs/opdelta
RuntimeResourceDictionaryValues-237.6k ± 0%37.6k ± 0%~(p=0.592 n=7+7)
RuntimeFungibleTokenTransfer-24.58k ± 0%4.58k ± 0%~(p=0.223 n=7+7)
Transfer-21.00 ± 0%1.00 ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-22.00 ± 0%2.00 ± 0%~(all equal)
ContractInterfaceFungibleToken-2458 ± 0%458 ± 0%~(all equal)
CheckContractInterfaceFungibleTokenConformance-21.07k ± 0%1.07k ± 0%~(all equal)
NewInterpreter/new_interpreter-213.0 ± 0%13.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-240.0 ± 0%40.0 ± 0%~(all equal)
InterpretRecursionFib-223.8k ± 0%23.8k ± 0%~(all equal)
 

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.

Thank you!

@turbolent
Copy link
Member

turbolent commented Mar 25, 2022

@sideninja While at it, maybe we can also bump to the latest Cadence release (https://github.com/onflow/cadence/releases/v0.23.2)?

@devbugging
Copy link
Contributor Author

@sideninja While at it, maybe we can also bump to the latest Cadence release (https://github.com/onflow/cadence/releases/v0.23.2)?

Yes ofc.

@devbugging
Copy link
Contributor Author

@turbolent sorry for delay, had some things come in between. Here is the version bump as well as other dependency updates.

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.

Thank you for updating this!

@turbolent
Copy link
Member

@sideninja To please CI, I think you have to merge master into this branch to get the semgrep config, and run go mod tidy to clean up the go.sum file

@turbolent turbolent merged commit 8060502 into master Apr 13, 2022
@turbolent turbolent deleted the gregor/ls-flowkit-update branch April 13, 2022 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants