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

refactor: Remove some type hell with SchemaTypeLinks #1837

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Jun 25, 2024

Summary

Minor refactor to improve the type hell when dealing with schemas.

Part A

Builds on top of #1836 (will need merging after)

Notably it:

  • Swaps CustomSchema::CustomTypeKind<L> for explicit CustomSchema::CustomLocalTypeKind and CustomSchema::AggregatorLocalTypeKind which simplifies a lot
  • Introduces some aliases to make using these types much less boilerplatey
pub type LocalTypeKind<S> = TypeKind<<S as CustomSchema>::CustomLocalTypeKind, LocalTypeId>;
pub type AggregatorTypeKind<S> =
    TypeKind<<S as CustomSchema>::CustomAggregatorTypeKind, RustTypeId>;

pub type LocalTypeData<S> = TypeData<<S as CustomSchema>::CustomLocalTypeKind, LocalTypeId>;
pub type AggregatorTypeData<S> =
    TypeData<<S as CustomSchema>::CustomAggregatorTypeKind, RustTypeId>;

Part B

Adds a CustomSchema::DefaultCustomExtension which we can use for encoding/decoding the schemas, which allows us to remove lots of really ugly/confusing bounds around the specific extension which we use for schema comparison, allowing us to just talk in terms of CustomSchema - because the schemas now know how to encode/decode themselves against the default extension.

The default extension for ScryptoCustomSchema is now ScryptoExtension (chosen over ManifestExtension for sanity).

Update Recommendations

If you get a compile error, change:

  • CustomSchema::CustomTypeKind<LocalTypeId> to CustomSchema::CustomLocalTypeKind
  • CustomSchema::CustomTypeKind<RustTypeId> to CustomSchema::AggregatorLocalTypeKind

Copy link

github-actions bot commented Jun 25, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:8eca17cafe

Copy link

github-actions bot commented Jun 25, 2024

Benchmark for 8eca17c

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 66.3±0.10ms 66.1±0.32ms -0.30%
costing::decode_sbor 11.1±0.02µs 11.2±0.01µs +0.90%
costing::decode_sbor_bytes 29.3±0.10µs 29.4±0.65µs +0.34%
costing::deserialize_wasm 1293.3±4.88µs 1310.7±3.73µs +1.35%
costing::instantiate_flash_loan 3.9±0.64ms 3.9±0.65ms 0.00%
costing::instantiate_radiswap 5.6±0.04ms 5.6±0.08ms 0.00%
costing::spin_loop 22.1±0.04ms 22.1±0.07ms 0.00%
costing::validate_sbor_payload 32.2±0.18µs 33.3±0.07µs +3.42%
costing::validate_sbor_payload_bytes 256.9±0.86ns 267.8±1.20ns +4.24%
costing::validate_secp256k1 76.4±0.17µs 76.3±0.06µs -0.13%
costing::validate_wasm 36.7±0.05ms 36.6±0.05ms -0.27%
decimal::add/0 8.4±0.00ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.9±0.00ns 9.9±0.02ns 0.00%
decimal::add/wasmer 114.7±0.07ns 113.5±0.14ns -1.05%
decimal::add/wasmer-call-native 454.8±0.77ns 451.1±0.57ns -0.81%
decimal::add/wasmi 666.9±6.47ns 571.1±1.68ns -14.36%
decimal::add/wasmi-call-native 5.2±0.01µs 5.0±0.01µs -3.85%
decimal::div/0 191.0±0.04ns 190.1±0.21ns -0.47%
decimal::from_string/0 154.9±0.17ns 152.9±0.16ns -1.29%
decimal::mul/0 141.7±0.06ns 140.2±0.21ns -1.06%
decimal::mul/rust-native 135.3±0.28ns 137.9±0.09ns +1.92%
decimal::mul/wasmer 1493.2±1.29ns 1532.1±1.92ns +2.61%
decimal::mul/wasmer-call-native 580.9±0.33ns 587.6±0.52ns +1.15%
decimal::mul/wasmi 40.6±0.10µs 40.0±0.08µs -1.48%
decimal::mul/wasmi-call-native 5.2±0.00µs 5.2±0.01µs 0.00%
decimal::pow/0 665.9±0.57ns 661.8±2.01ns -0.62%
decimal::pow/rust-native 629.5±0.84ns 626.9±0.63ns -0.41%
decimal::pow/wasmer 6.6±0.01µs 6.6±0.00µs 0.00%
decimal::pow/wasmer-call-native 1024.8±0.61ns 1020.0±0.57ns -0.47%
decimal::pow/wasmi 193.8±0.34µs 193.8±0.52µs 0.00%
decimal::pow/wasmi-call-native 5.2±0.00µs 5.0±0.02µs -3.85%
decimal::root/0 8.3±0.20µs 8.0±0.20µs -3.61%
decimal::sub/0 8.5±0.01ns 8.5±0.01ns 0.00%
decimal::to_string/0 466.7±0.83ns 447.1±6.40ns -4.20%
precise_decimal::add/0 9.3±0.28ns 9.3±0.08ns 0.00%
precise_decimal::add/rust-native 11.4±0.01ns 11.4±0.01ns 0.00%
precise_decimal::add/wasmer 122.8±0.06ns 121.0±0.05ns -1.47%
precise_decimal::add/wasmer-call-native 492.6±0.65ns 494.7±1.58ns +0.43%
precise_decimal::add/wasmi 821.2±5.15ns 730.6±2.81ns -11.03%
precise_decimal::add/wasmi-call-native 6.5±0.01µs 6.4±0.01µs -1.54%
precise_decimal::div/0 301.1±0.15ns 309.2±0.50ns +2.69%
precise_decimal::from_string/0 205.2±0.39ns 199.5±0.31ns -2.78%
precise_decimal::mul/0 348.4±1.22ns 354.7±0.64ns +1.81%
precise_decimal::mul/rust-native 304.9±0.55ns 304.3±0.54ns -0.20%
precise_decimal::mul/wasmer 3.4±0.00µs 3.4±0.01µs 0.00%
precise_decimal::mul/wasmer-call-native 849.4±1.67ns 843.3±0.96ns -0.72%
precise_decimal::mul/wasmi 103.8±0.18µs 102.9±0.13µs -0.87%
precise_decimal::mul/wasmi-call-native 6.9±0.02µs 6.9±0.01µs 0.00%
precise_decimal::pow/0 1914.8±6.89ns 1856.0±3.99ns -3.07%
precise_decimal::pow/rust-native 1464.5±1.97ns 1483.6±2.43ns +1.30%
precise_decimal::pow/wasmer 16.1±0.01µs 16.1±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 501.6±0.87µs 502.9±1.97µs +0.26%
precise_decimal::pow/wasmi-call-native 12.8±0.03µs 12.5±0.03µs -2.34%
precise_decimal::root/0 56.6±0.04µs 57.4±0.02µs +1.41%
precise_decimal::sub/0 9.5±0.00ns 9.5±0.00ns 0.00%
precise_decimal::to_string/0 719.6±1.01ns 796.5±4.60ns +10.69%
schema::validate_payload 367.2±0.88µs 367.9±0.82µs +0.19%
transaction::radiswap 5.4±0.02ms 5.5±0.02ms +1.85%
transaction::transfer 1806.2±3.66µs 1810.5±5.92µs +0.24%
transaction_processing::prepare 2.6±0.01ms 2.5±0.00ms -3.85%
transaction_processing::prepare_and_decompile 6.4±0.02ms 6.4±0.02ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 26.4±1.70ms 24.2±0.25ms -8.33%
transaction_validation::validate_manifest 42.4±0.51µs 42.1±0.03µs -0.71%
transaction_validation::verify_bls_2KB 1037.3±87.30µs 969.1±15.07µs -6.57%
transaction_validation::verify_bls_32B 969.1±12.43µs 965.8±14.35µs -0.34%
transaction_validation::verify_ecdsa 74.2±0.07µs 74.2±0.04µs 0.00%
transaction_validation::verify_ed25519 54.9±0.04µs 55.3±1.15µs +0.73%

@dhedey dhedey merged commit af02c9a into feature/schema-comparisons Jun 27, 2024
28 of 29 checks passed
@dhedey
Copy link
Contributor Author

dhedey commented Jun 27, 2024

Merged into the underlying PR at Yulong's request

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.

1 participant