stacks-core
stacks-core copied to clipboard
Costs-3 contract
Description
This PR adds new cost functions for the Clarity2 functions, and adds the costs-3 contract.
For the most part, the new costs contract does not alter old costs. However, as part of this benchmarking effort, I revamped the mocked data used (database and MARF accesses are now much more realistic). I also caught a few errors related to the computation of the input size (which only significantly altered one cost function). These changes led to a few cost changes. I provide a list of the altered costs below:
- cost_stx_balance (increased 7x) - 99% of the runtime is in
get_stx_balance_snapshot(DB access) - cost_stx_transfer (increased 7x) - 99% of the runtime is in
get_stx_balance_snapshot(DB access) - cost_at_block (increased 10x) - using a real block now, which explains cost increase
- cost_ast_parse (decreased 6x) (improvements in v2)
- cost_concat (decreased 2x). (fixed input size computation)
We are now using version 0.21.0 of the Rust secp256k1 library. When we benchmarked the costs-2 contract, the version of this library was at 0.19.0. Due to improvements in these libraries, the runtimes for the following functions changed.
- cost_secp256k1recover (decreased 2x)
- cost_secp256k1verify (decreased 2x)
- cost_poison_microblock (decreased 2x) - this function recovers the public key (so it essentially does the same operation as secp256k1recover)
Applicable issues
- fixes #3115
Additional info (benefits, drawbacks, caveats)
- Not currently using new cost functions in the code base as that requires additional code change of loading the costs-3 contract (#3266) - I can also address that in this PR if people feel that is appropriate.
Codecov Report
Merging #3267 (4156983) into next (557f184) will decrease coverage by
0.03%. The diff coverage is19.47%.
@@ Coverage Diff @@
## next #3267 +/- ##
==========================================
- Coverage 29.60% 29.56% -0.04%
==========================================
Files 279 284 +5
Lines 251259 258262 +7003
==========================================
+ Hits 74378 76354 +1976
- Misses 176881 181908 +5027
| Impacted Files | Coverage Δ | |
|---|---|---|
| clarity/src/vm/functions/assets.rs | 26.96% <0.00%> (ø) |
|
| clarity/src/vm/functions/conversions.rs | 14.56% <0.00%> (-0.40%) |
:arrow_down: |
| clarity/src/vm/functions/principals.rs | 0.00% <0.00%> (ø) |
|
| clarity/src/vm/functions/sequences.rs | 50.98% <0.00%> (-2.77%) |
:arrow_down: |
| clarity/src/vm/types/mod.rs | 50.41% <0.00%> (-0.28%) |
:arrow_down: |
| src/burnchains/tests/affirmation.rs | 0.00% <0.00%> (ø) |
|
| src/burnchains/tests/burnchain.rs | 0.00% <0.00%> (ø) |
|
| src/burnchains/tests/db.rs | 0.00% <0.00%> (ø) |
|
| src/burnchains/tests/mod.rs | 0.00% <0.00%> (ø) |
|
| .../chainstate/burn/operations/leader_key_register.rs | 15.39% <0.00%> (-0.03%) |
:arrow_down: |
| ... and 92 more |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
cost_stx_balance (increased 7x) - 99% of the runtime is in get_stx_balance_snapshot (DB access) cost_stx_transfer (increased 7x) - 99% of the runtime is in get_stx_balance_snapshot (DB access) cost_at_block (increased 10x) - using a real block now, which explains cost increase
Wow! :exploding_head: Do we know if there's a way to speed these operations up? Curious if this is due to a natural average MARF key-path lengthening that will naturally occur at a logarithmic scale as more data gets added, or if this is due to an artifact / regression of the reference implementation.
Notes from the blockchain meeting:
- This PR tests runtimes against a real DB, so the runtime cost includes the I/O costs of hitting a real MARF. Before, we had determined the read_count and write_count values with a real MARF, but determined the runtime values with a mocked MARF.
@pavitthrap raised a good point about the advantages of using the real backend versus the mocked backend -- this kind of relates to a question about the size of the MARF used in the benchmarks.
Do you know approximately how many keys were in the MARF, and how many MARF "blocks" there are in the database?
Update: will replace the MARF with a smaller one (up to 10 blocks).
Will re-benchmark the functions with the new MARF once the PR for the function replace-at is done.
Need a function for replace-at and the new analysis function for trait-checking
This PR tests runtimes against a real DB, so the runtime cost includes the I/O costs of hitting a real MARF. Before, we had determined the read_count and write_count values with a real MARF, but determined the runtime values with a mocked MARF.
@pavitthrap does the benchmark with real MARF use HDD or SSD?
- It appears https://github.com/stacks-network/stacks-blockchain/issues/3266 is needed for 2.1. Could we include it to this PR?
- Do we have a programmatic check that the ClarityCostFunction enum is complete and not missing clarity functions?
This PR tests runtimes against a real DB, so the runtime cost includes the I/O costs of hitting a real MARF. Before, we had determined the read_count and write_count values with a real MARF, but determined the runtime values with a mocked MARF.
@pavitthrap does the benchmark with real MARF use HDD or SSD?
It uses a standard persistent disk, so HDD.
This should also address #3380 and #3381.
- It appears Load costs-3 contract #3266 is needed for 2.1. Could we include it to this PR?
- Do we have a programmatic check that the ClarityCostFunction enum is complete and not missing clarity functions?
I'll be addressing #3380 and #3381, so I'll see about adding the contract when I do that. I added a programmatic check now.
N.B. the benchmarks for I/O should use an SSD, because we don't want seeks and filesystem implementation details to interfere with benchmarks.
Okay @gregorycoppola @obycode @igorsyl @a3slade, I've made a few optimizations to this PR so that the cost functions for the stx-* functions are now only about 20% slower instead of over 8x slower. The reason for this is because we have to determine the current burnchain tip height in order to calculate the consolidated balance, which incurs some extra CPU cycles and I/O for loading up the data from the headers DB and parsing it out.
Once #3389 merges, I'll go and add cost functions for the bitwise operators.
This PR also addresses #3266. costs-3 is loaded as part of the epoch 2.1 instantiation.
In addition, this PR addresses #3381, #3380, and #3311.
Okay, had to revert the implementation of ClarityDB::get_current_burnchain_block_height() because the new one was buggy. But, the other optimizations can remain in place. The stx-* methods are now 2x slower than they were in 2.05, due to the additional burnchain DB MARF query.
transition_empty_blocks test failing