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

Fixing the SystemDbReader's handling of Object's Index/SortedIndex entry look-up #1662

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

jakrawcz-rdx
Copy link
Contributor

This is a fix for a problem I encountered during development of Browse API's "get object collection entry" (see https://github.com/radixdlt/babylon-node/pull/755/files#diff-0a2c5a225e361800cb28568a6b51d0c0bbe1c5b5aded21ca10c783ef6fc0f743R174).

The SystemDatabaseReader was handling ObjectCollectionKey::KeyValue case just fine (i.e. it expected KeyValueEntrySubstate<V> values). However, for ObjectCollectionKey::Index/SortedIndex cases, it wrongfully expected the V values (without IndexEntrySubstate<V>/SortedIndexEntrySubstate<V> wrappers).

I unfortunately had to add the "unit" test in the radix-engine-tests (it cannot live next to SystemDatabaseReader since the SubstateDatabase impl required to run it is not available there).
(In general, the SystemDatabaseReader seems to have been developed as a test util [not covered by its own tests, but tested indirectly by assertions in tests of higher-level features], and I was not trying to change it in this PR)

Copy link

github-actions bot commented Dec 4, 2023

Benchmark for 4b3dc78

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 65.8±0.34ms 65.1±0.32ms -1.06%
costing::decode_sbor 13.2±0.02µs 13.0±0.02µs -1.52%
costing::decode_sbor_bytes 37.0±0.02µs 35.6±0.02µs -3.78%
costing::deserialize_wasm 1253.6±1.42µs 1255.6±2.74µs +0.16%
costing::instantiate_flash_loan 3.8±0.12ms 3.9±0.30ms +2.63%
costing::instantiate_radiswap 5.3±0.08ms 5.2±0.06ms -1.89%
costing::spin_loop 23.1±0.01ms 23.3±0.01ms +0.87%
costing::validate_sbor_payload 25.6±0.02µs 25.9±0.02µs +1.17%
costing::validate_sbor_payload_bytes 358.1±0.65ns 343.8±0.19ns -3.99%
costing::validate_secp256k1 80.4±0.06µs 80.5±0.08µs +0.12%
costing::validate_wasm 35.6±0.06ms 35.7±0.04ms +0.28%
decimal::add/0 7.2±0.00ns 7.2±0.00ns 0.00%
decimal::add/rust-native 9.2±0.05ns 9.1±0.07ns -1.09%
decimal::add/wasmer 139.8±0.26ns 139.1±0.10ns -0.50%
decimal::add/wasmer-call-native 599.7±0.29ns 585.0±0.23ns -2.45%
decimal::add/wasmi 478.4±1.45ns 474.7±10.25ns -0.77%
decimal::add/wasmi-call-native 6.1±0.02µs 5.8±0.02µs -4.92%
decimal::div/0 165.5±0.10ns 166.5±0.12ns +0.60%
decimal::from_string/0 145.1±0.08ns 145.7±0.08ns +0.41%
decimal::mul/0 128.9±0.11ns 128.6±0.10ns -0.23%
decimal::mul/rust-native 132.9±0.05ns 131.8±0.06ns -0.83%
decimal::mul/wasmer 1711.6±1.64ns 1709.6±0.94ns -0.12%
decimal::mul/wasmer-call-native 716.1±0.21ns 713.0±0.37ns -0.43%
decimal::mul/wasmi 29.9±0.06µs 30.2±0.03µs +1.00%
decimal::mul/wasmi-call-native 6.2±0.02µs 6.1±0.02µs -1.61%
decimal::pow/0 624.1±0.23ns 620.7±0.29ns -0.54%
decimal::pow/rust-native 605.7±0.29ns 607.5±0.23ns +0.30%
decimal::pow/wasmer 7.5±0.00µs 7.5±0.01µs 0.00%
decimal::pow/wasmer-call-native 1129.0±0.99ns 1121.4±0.19ns -0.67%
decimal::pow/wasmi 140.7±0.27µs 144.7±0.17µs +2.84%
decimal::pow/wasmi-call-native 5.8±0.03µs 5.4±0.03µs -6.90%
decimal::root/0 9.1±0.00µs 9.3±0.00µs +2.20%
decimal::sub/0 7.2±0.01ns 7.2±0.00ns 0.00%
decimal::to_string/0 494.0±0.44ns 493.6±0.22ns -0.08%
precise_decimal::add/0 8.0±0.00ns 8.0±0.00ns 0.00%
precise_decimal::add/rust-native 10.4±0.02ns 10.4±0.01ns 0.00%
precise_decimal::add/wasmer 146.8±0.10ns 145.6±0.05ns -0.82%
precise_decimal::add/wasmer-call-native 608.9±0.25ns 606.1±0.12ns -0.46%
precise_decimal::add/wasmi 577.2±1.22ns 559.3±0.14ns -3.10%
precise_decimal::add/wasmi-call-native 6.5±0.02µs 6.2±0.03µs -4.62%
precise_decimal::div/0 264.4±0.06ns 261.5±0.09ns -1.10%
precise_decimal::from_string/0 201.7±0.07ns 202.4±0.11ns +0.35%
precise_decimal::mul/0 283.9±0.18ns 286.0±0.15ns +0.74%
precise_decimal::mul/rust-native 259.9±0.47ns 261.0±0.07ns +0.42%
precise_decimal::mul/wasmer 4.0±0.00µs 4.0±0.00µs 0.00%
precise_decimal::mul/wasmer-call-native 900.8±0.43ns 899.4±0.32ns -0.16%
precise_decimal::mul/wasmi 78.7±0.15µs 78.6±0.02µs -0.13%
precise_decimal::mul/wasmi-call-native 6.8±0.02µs 6.5±0.03µs -4.41%
precise_decimal::pow/0 1574.3±0.37ns 1596.1±0.31ns +1.38%
precise_decimal::pow/rust-native 1265.8±0.16ns 1265.6±0.43ns -0.02%
precise_decimal::pow/wasmer 18.9±0.01µs 18.9±0.02µs 0.00%
precise_decimal::pow/wasmer-call-native 2.1±0.00µs 2.1±0.00µs 0.00%
precise_decimal::pow/wasmi 379.5±0.29µs 380.1±0.42µs +0.16%
precise_decimal::pow/wasmi-call-native 11.2±0.02µs 11.1±0.03µs -0.89%
precise_decimal::root/0 59.9±0.02µs 60.5±0.01µs +1.00%
precise_decimal::sub/0 8.3±0.00ns 8.3±0.00ns 0.00%
precise_decimal::to_string/0 747.0±0.26ns 748.4±0.26ns +0.19%
schema::validate_payload 285.3±0.20µs 284.1±0.26µs -0.42%
transaction::radiswap 5.4±0.04ms 5.4±0.05ms 0.00%
transaction::transfer 1752.1±3.71µs 1739.4±3.60µs -0.72%
transaction_processing::prepare 2.5±0.00ms 2.5±0.00ms 0.00%
transaction_processing::prepare_and_decompile 6.4±0.01ms 6.4±0.03ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 24.1±0.11ms 23.9±0.09ms -0.83%
transaction_validation::validate_manifest 44.1±0.05µs 44.1±0.05µs 0.00%
transaction_validation::verify_ecdsa 78.0±0.09µs 78.0±0.06µs 0.00%
transaction_validation::verify_ed25519 52.8±0.07µs 52.7±0.03µs -0.19%

@jakrawcz-rdx jakrawcz-rdx merged commit 3159217 into develop Dec 5, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants