refactor: prepare and dump state inside `params-estimator` #4706

Longarithm · 2021-08-17T23:45:33Z

Currently we prepare and dump state using the separate genesis-populate call:

cargo run --release --package genesis-populate --bin genesis-populate -- --additional-accounts-num 200000 --home /tmp/data

We can move this to the params-estimator, so we don't need to call a separate script, which simplifies the workflow.
Now the params-estimator call should look as follows:

cargo run --release --package runtime-params-estimator --features required --bin runtime-params-estimator -- --home /tmp/data --accounts-num 20000 --additional-accounts-num 200000 --iters 1 --warmup-iters 1 --metric time

Test Plan

cargo check
TODO: Ensure that resulting RuntimeConfig is the same.

matklad · 2021-08-19T12:25:36Z

runtime/runtime-params-estimator/src/main.rs

+    /// Number of additional accounts to add to the state, among which active accounts are selected.
+    #[clap(long)]
+    additional_accounts_num: Option<usize>,


It feels like this should have a deafult_value -- we always want to populate genesis with some amount of additional accounts, no?

The problem is that we want to execute it before launching docker, and after that, we need to drop this argument, because we shouldn't populate genesis state twice. Option seems to be the simplest solution here.

Good question. Let's just use 0 as "don't run genesis populaet", and add that arg to --docker mode? If we need this elsewhere, we might add an extra flag for this.

I do think that we need a default here, so that run without args does the right thing.

genesis-tools/genesis-populate/src/main.rs

Longarithm · 2021-08-23T11:56:05Z

Also added cd /host/nearcore for docker mode, so that costs table could be actually accessed

runtime/runtime-params-estimator/src/main.rs

Longarithm · 2021-08-23T19:17:59Z

Cost table after this change:

ActionReceiptCreation                         343_222_250_000
ActionSirReceiptCreation                      256_250_125_000
DataReceiptCreationBase                        66_851_060_125
DataReceiptCreationPerByte                         52_040_585
ActionCreateAccount                            70_696_375_000
ActionDeployContractBase                    2_335_831_894_262
ActionDeployContractPerByte                       417_715_605
ActionFunctionCallBase                        382_498_250_000
ActionFunctionCallPerByte                              60_521
ActionTransfer                                 59_634_875_000
ActionStake                                   128_057_875_000
ActionAddFullAccessKey                         99_077_000_000
ActionAddFunctionAccessKeyBase                 69_860_500_000
ActionAddFunctionAccessKeyPerByte                  25_575_062
ActionDeleteKey                                33_510_625_000
ActionDeleteAccount                           271_664_875_000
HostFunctionCall                                  151_694_418
WasmInstruction                                     2_290_268
ReadMemoryBase                                    336_591_237
ReadMemoryByte                                      1_270_371
WriteMemoryBase                                   411_759_075
WriteMemoryByte                                        64_383
ReadRegisterBase                                  299_263_937
ReadRegisterByte                                       34_991
WriteRegisterBase                                 451_203_037
WriteRegisterByte                                   1_270_479
Utf8DecodingBase                                  558_304_562
Utf8DecodingByte                                  124_619_874
Utf16DecodingBase                                 716_354_900
Utf16DecodingByte                                  67_973_585
Sha256Base                                      1_128_982_275
Sha256Byte                                          9_542_620
Keccak256Base                                   1_476_523_337
Keccak256Byte                                       7_117_001
Keccak512Base                                   1_454_847_712
Keccak512Byte                                      12_182_568
Ripemd160Base                                     825_552_862
Ripemd160Block                                    313_588_944
EcrecoverBase                                  93_210_545_525
LogBase                                           716_354_900
LogByte                                             4_130_096
StorageWriteBase                               16_625_867_625
StorageWriteKeyByte                                25_109_230
StorageWriteValueByte                              14_199_723
StorageWriteEvictedByte                            13_256_129
StorageReadBase                                23_820_780_500
StorageReadKeyByte                                 11_189_981
StorageReadValueByte                                2_622_705
StorageRemoveBase                              32_697_171_500
StorageRemoveKeyByte                               23_532_238
StorageRemoveRetValueByte                           3_420_196
StorageHasKeyBase                              22_479_269_000
StorageHasKeyByte                                  10_844_708
TouchingTrieNode                                6_805_937_285
PromiseAndBase                                    558_485_745
PromiseAndPerPromise                                2_097_221
PromiseReturn                                     213_633_491

bowenwang1996 · 2021-08-23T22:09:26Z

@Longarithm it seems that action receipt creation has become much more expensive. What caused it to change?

Longarithm · 2021-08-23T22:40:21Z

@Longarithm it seems that action receipt creation has become much more expensive. What caused it to change?

This is the case since work on new fees started #4455 (comment) (second/first ratio is also around 1.5). I didn't look at it yet.

bowenwang1996 · 2021-08-24T18:17:36Z

second/first ratio is also around 1.5

It is more than 1.5 compared to the current cost right? We have 343_222_250_000 as you posted above and that is more than 30 times the current cost

Longarithm · 2021-08-24T18:42:32Z

It is more than 1.5 compared to the current cost right? We have 343_222_250_000 as you posted above and that is more than 30 times the current cost

The current cost taken from mainnet_genesis is 108_059_500_000 which is twice lower than cost from CostTable - 216_119_000_000, as expected.

Longarithm · 2021-08-24T20:15:54Z

The only significant difference, got by modified script from here #4729:

nearcore % python runtime/runtime-params-estimator/emu-cost/compare_costs.py ~/tmp/cost_table_old.txt ~/tmp/cost_table_new.txt --type table --diff
ActionCreateAccount: first=51167000000 second=70696375000 second/first=1.38

Re-run estimation for only ActionCreateAccount showed that 70_000_000_000 is more accurate. Need to re-run params-estimator from master to check volatility of this metric.

Full cost table before the change

ActionReceiptCreation                         352_834_375_000
ActionSirReceiptCreation                      257_010_750_000
DataReceiptCreationBase                        66_932_501_250
DataReceiptCreationPerByte                         52_016_955
ActionCreateAccount                            51_167_000_000
ActionDeployContractBase                    2_360_166_058_188
ActionDeployContractPerByte                       417_602_419
ActionFunctionCallBase                        380_294_625_000
ActionFunctionCallPerByte                              53_576
ActionTransfer                                 51_918_625_000
ActionStake                                   130_251_875_000
ActionAddFullAccessKey                         96_060_125_000
ActionAddFunctionAccessKeyBase                 66_971_625_000
ActionAddFunctionAccessKeyPerByte                  29_267_500
ActionDeleteKey                                32_577_500_000
ActionDeleteAccount                           275_795_625_000
HostFunctionCall                                  151_661_448
WasmInstruction                                     2_290_284
ReadMemoryBase                                    333_773_175
ReadMemoryByte                                      1_270_368
WriteMemoryBase                                   408_563_400
WriteMemoryByte                                        64_381
ReadRegisterBase                                  295_737_475
ReadRegisterByte                                       34_988
ActionStake                                   130_251_875_000
ActionAddFullAccessKey                         96_060_125_000
ActionAddFunctionAccessKeyBase                 66_971_625_000
ActionAddFunctionAccessKeyPerByte                  29_267_500
ActionDeleteKey                                32_577_500_000
ActionDeleteAccount                           275_795_625_000
HostFunctionCall                                  151_661_448
WasmInstruction                                     2_290_284
ReadMemoryBase                                    333_773_175
ReadMemoryByte                                      1_270_368
WriteMemoryBase                                   408_563_400
WriteMemoryByte                                        64_381
ReadRegisterBase                                  295_737_475
ReadRegisterByte                                       34_988
WriteRegisterBase                                 447_455_962
WriteRegisterByte                                   1_270_318
Utf8DecodingBase                                  554_633_737
Utf8DecodingByte                                  124_624_117
Utf16DecodingBase                                 713_177_287
Utf16DecodingByte                                  67_997_169
Sha256Base                                      1_124_883_737
Sha256Byte                                          9_558_394
Keccak256Base                                   1_473_411_262
Keccak256Byte                                       7_116_354
Keccak512Base                                   1_451_739_162
Keccak512Byte                                      12_182_152
Ripemd160Base                                     821_670_825
Ripemd160Block                                    313_562_535
EcrecoverBase                                  93_205_777_825
LogBase                                           713_177_287
LogByte                                             4_143_153
StorageWriteBase                               17_034_470_375
StorageWriteKeyByte                                25_151_330
StorageWriteValueByte                              14_176_926
StorageWriteEvictedByte                            13_254_253
StorageReadBase                                25_043_735_750
StorageReadKeyByte                                 11_109_528
StorageReadValueByte                                2_533_503
StorageRemoveBase                              33_628_840_500
StorageRemoveKeyByte                               23_496_485
StorageRemoveRetValueByte                           3_589_315
StorageHasKeyBase                              22_986_364_625
StorageHasKeyByte                                  10_923_992
TouchingTrieNode                                7_155_353_071
PromiseAndBase                                    558_206_745
PromiseAndPerPromise                                2_093_144
PromiseReturn                                     213_327_461

matklad · 2021-08-25T09:35:00Z

I don't think it makes sense to investigate cost discrepancy here. We don't compare before/after this PR, we compare this PR with the current costs, which, eg, don't account for IO. As is, I think most of the difference will show up even when we compare master with this PR (#4474).

My plan there is to refactor estimator first (so that we can easily run it, and understand deps between the cost), and then holistically look at the diff of the resulting costs.

That being said, the output in the table includes actual costs -- they are not multiplied by the safety multiplier, and they are not split into sender/receiver fees. They should be compared with this file, and indeed there's 1.5x difference there. I'll add relative diff to the default output, but only after #4474, so that the diffs are not misleading!

matklad · 2021-08-25T09:37:28Z

.buildkite/pipeline.yml

@@ -96,8 +96,7 @@ steps:
      ./build.sh
      cd ..
      RUSTFLAGS='-D warnings' cargo run --release --package neard --bin neard -- --home /tmp/data init --test-seed=alice.near --account-id=test.near --fast
-      RUSTFLAGS='-D warnings' cargo run --release --package genesis-populate --bin genesis-populate -- --additional-accounts-num=200000 --home /tmp/data
-      RUSTFLAGS='-D warnings' cargo run --release --package runtime-params-estimator --bin runtime-params-estimator -- --home /tmp/data --accounts-num 20000 --iters 1 --warmup-iters 1 --metric time
+      RUSTFLAGS='-D warnings' cargo run --release --package runtime-params-estimator --bin runtime-params-estimator -- --home /tmp/data --accounts-num 20000 --additional-accounts-num 200000 --iters 1 --warmup-iters 1 --metric time


matklad · 2021-08-25T09:38:50Z

runtime/runtime-params-estimator/Cargo.toml

@@ -30,6 +30,8 @@ near-vm-runner = {path = "../../runtime/near-vm-runner" }
 node-runtime = { path = "../../runtime/runtime" }
 near-store = { path = "../../core/store" }
 near-primitives = { path = "../../core/primitives" }
+near-test-contracts = { path = "../../runtime/near-test-contracts" }


Hm, I am surprised that estimator uses both near-test-contracts and it's own test-contract. This is unrelated to the PR

matklad · 2021-08-25T09:39:08Z

runtime/runtime-params-estimator/emu-cost/README.md

@@ -32,15 +32,14 @@ Start container and build estimator with:
    host> ./run.sh
    docker> cd /host/nearcore
    docker> cargo run -j2 --release --package neard --bin neard -- --home /tmp/data init --test-seed=alice.near --account-id=test.near --fast
-    docker> cargo run -j2 --release --package genesis-populate --bin genesis-populate -- --additional-accounts-num=200000 --home /tmp/data


Thanks for remebering to update the docs!

matklad · 2021-08-25T09:39:19Z

runtime/runtime-params-estimator/src/main.rs

@@ -146,6 +170,7 @@ fn main_docker(state_dump_path: &Path) -> anyhow::Result<()> {

        let mut buf = String::new();
        buf.push_str("set -ex;\n");
+        buf.push_str("cd /host/nearcore;\n");


matklad · 2021-08-25T09:41:40Z

~~Going to merge this as I think we explained what's going on with the actual costs here.~~ not so fast, some more reviews are required!

Longarithm added 5 commits August 18, 2021 02:12

add additional accounts inside params-estimator

350209f

fix args

b1cbdd9

cargo fmt

8cfc449

fix docs

2f898f2

fix

fccd259

Longarithm changed the title ~~Prepare and dump state inside params-estimator~~ Prepare and dump state inside params-estimator Aug 17, 2021

move genesis-populate above

e17c864

Longarithm changed the title ~~Prepare and dump state inside params-estimator~~ refactor: prepare and dump state inside params-estimator Aug 19, 2021

Longarithm and others added 5 commits August 19, 2021 14:01

fix skip arg

8809c03

fix arg name

fbd16f2

minor readme fix

35969ad

compilation fix

a7b18c1

Merge branch 'master' into inject-genesis

f4cff16

Longarithm marked this pull request as ready for review August 19, 2021 11:39

Longarithm requested review from ailisp, matklad and olonho as code owners August 19, 2021 11:39

matklad reviewed Aug 19, 2021

View reviewed changes

Longarithm and others added 6 commits August 20, 2021 22:31

apply suggestions

40233c0

compile fix

6fc105a

compile fix

1b8b2ce

compile fix

b531f84

fix

7879ee9

Merge branch 'master' into inject-genesis

f24da54

matklad reviewed Aug 23, 2021

View reviewed changes

runtime/runtime-params-estimator/src/main.rs Outdated Show resolved Hide resolved

Longarithm added 3 commits August 23, 2021 16:19

set 0 in docker

c6bed4a

address suggestion

17c2223

cargo fmt

e3f2811

fix pipeline

def0387

Longarithm requested review from chefsale and mhalambek as code owners August 23, 2021 19:20

Merge branch 'master' into inject-genesis

3e3a7ac

matklad approved these changes Aug 25, 2021

View reviewed changes

matklad added S-automerge and removed S-automerge labels Aug 25, 2021

Longarithm force-pushed the inject-genesis branch from b59290c to 3e3a7ac Compare August 25, 2021 13:22

revert genesis

9bf5a64

mhalambek approved these changes Aug 25, 2021

View reviewed changes

Merge branch 'master' into inject-genesis

92b24d7

Longarithm self-assigned this Aug 25, 2021

Longarithm added A-params-estimator Area: runtime params estimator S-automerge labels Aug 25, 2021

near-bulldozer bot merged commit 47bf35b into master Aug 25, 2021

near-bulldozer bot deleted the inject-genesis branch August 25, 2021 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: prepare and dump state inside `params-estimator` #4706

refactor: prepare and dump state inside `params-estimator` #4706

Longarithm commented Aug 17, 2021 •

edited

Loading

matklad Aug 19, 2021

Longarithm Aug 23, 2021

matklad Aug 23, 2021

Longarithm commented Aug 23, 2021

Longarithm commented Aug 23, 2021

bowenwang1996 commented Aug 23, 2021

Longarithm commented Aug 23, 2021

bowenwang1996 commented Aug 24, 2021

Longarithm commented Aug 24, 2021

Longarithm commented Aug 24, 2021

matklad commented Aug 25, 2021

matklad Aug 25, 2021

matklad Aug 25, 2021

matklad Aug 25, 2021

matklad Aug 25, 2021

matklad commented Aug 25, 2021 •

edited

Loading

refactor: prepare and dump state inside params-estimator #4706

refactor: prepare and dump state inside params-estimator #4706

Conversation

Longarithm commented Aug 17, 2021 • edited Loading

Test Plan

matklad Aug 19, 2021

Choose a reason for hiding this comment

Longarithm Aug 23, 2021

Choose a reason for hiding this comment

matklad Aug 23, 2021

Choose a reason for hiding this comment

Longarithm commented Aug 23, 2021

Longarithm commented Aug 23, 2021

bowenwang1996 commented Aug 23, 2021

Longarithm commented Aug 23, 2021

bowenwang1996 commented Aug 24, 2021

Longarithm commented Aug 24, 2021

Longarithm commented Aug 24, 2021

matklad commented Aug 25, 2021

matklad Aug 25, 2021

Choose a reason for hiding this comment

matklad Aug 25, 2021

Choose a reason for hiding this comment

matklad Aug 25, 2021

Choose a reason for hiding this comment

matklad Aug 25, 2021

Choose a reason for hiding this comment

matklad commented Aug 25, 2021 • edited Loading

refactor: prepare and dump state inside `params-estimator` #4706

refactor: prepare and dump state inside `params-estimator` #4706

Longarithm commented Aug 17, 2021 •

edited

Loading

matklad commented Aug 25, 2021 •

edited

Loading