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

feature: Add type_name override to Describe proc macro #1671

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Jan 6, 2024

Summary

Adds functionality for type name override via #[sbor(type_name = "NewTypeName")].

This is an addition, not a breaking change.

This would allow e.g. us to rename types without affecting their scrypto schema. For example, we could create e.g. TimePrecisionV2 and rename TimePrecision to TimePrecisionV1, whilst allowing them both to have an SBOR type name of TimePrecision. The latter is required for backwards compatibility.

Testing

Tests are in sbor-tests.

Copy link

github-actions bot commented Jan 6, 2024

Benchmark for d7a020b

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 63.9±0.55ms 63.6±0.38ms -0.47%
costing::decode_sbor 13.1±0.01µs 13.2±0.03µs +0.76%
costing::decode_sbor_bytes 36.5±0.07µs 36.7±0.22µs +0.55%
costing::deserialize_wasm 1242.8±2.26µs 1245.4±1.77µs +0.21%
costing::instantiate_flash_loan 3.8±0.40ms 3.9±0.52ms +2.63%
costing::instantiate_radiswap 5.1±0.05ms 5.0±0.05ms -1.96%
costing::spin_loop 21.3±0.01ms 21.3±0.01ms 0.00%
costing::validate_sbor_payload 26.1±0.02µs 26.1±0.03µs 0.00%
costing::validate_sbor_payload_bytes 358.5±3.92ns 364.3±0.25ns +1.62%
costing::validate_secp256k1 80.3±0.05µs 80.2±0.06µs -0.12%
costing::validate_wasm 34.6±0.05ms 34.5±0.08ms -0.29%
decimal::add/0 7.2±0.00ns 7.2±0.00ns 0.00%
decimal::add/rust-native 9.4±0.03ns 9.4±0.02ns 0.00%
decimal::add/wasmer 134.2±0.12ns 135.9±0.11ns +1.27%
decimal::add/wasmer-call-native 520.6±0.19ns 524.1±0.27ns +0.67%
decimal::add/wasmi 447.5±0.64ns 440.8±0.05ns -1.50%
decimal::add/wasmi-call-native 3.3±0.01µs 3.4±0.01µs +3.03%
decimal::div/0 166.2±0.07ns 166.2±0.09ns 0.00%
decimal::from_string/0 154.7±0.06ns 155.0±0.11ns +0.19%
decimal::mul/0 129.4±0.06ns 128.3±0.06ns -0.85%
decimal::mul/rust-native 132.1±0.07ns 133.0±0.10ns +0.68%
decimal::mul/wasmer 1756.2±0.51ns 1745.5±0.75ns -0.61%
decimal::mul/wasmer-call-native 652.2±0.18ns 653.0±0.21ns +0.12%
decimal::mul/wasmi 26.1±0.04µs 26.5±0.03µs +1.53%
decimal::mul/wasmi-call-native 3.5±0.02µs 3.5±0.01µs 0.00%
decimal::pow/0 622.0±0.21ns 621.5±0.14ns -0.08%
decimal::pow/rust-native 609.0±0.33ns 610.1±0.55ns +0.18%
decimal::pow/wasmer 7.6±0.01µs 7.6±0.00µs 0.00%
decimal::pow/wasmer-call-native 1079.4±0.55ns 1075.5±1.36ns -0.36%
decimal::pow/wasmi 123.5±0.15µs 129.5±0.42µs +4.86%
decimal::pow/wasmi-call-native 3.4±0.02µs 3.4±0.01µs 0.00%
decimal::root/0 9.1±0.00µs 9.0±0.00µs -1.10%
decimal::sub/0 7.2±0.00ns 7.2±0.00ns 0.00%
decimal::to_string/0 489.6±0.11ns 490.4±0.32ns +0.16%
precise_decimal::add/0 8.0±0.00ns 8.0±0.00ns 0.00%
precise_decimal::add/rust-native 10.2±0.01ns 10.4±0.01ns +1.96%
precise_decimal::add/wasmer 141.2±0.20ns 140.6±0.10ns -0.42%
precise_decimal::add/wasmer-call-native 532.4±0.30ns 540.1±0.08ns +1.45%
precise_decimal::add/wasmi 535.8±1.04ns 530.1±1.23ns -1.06%
precise_decimal::add/wasmi-call-native 3.6±0.01µs 3.6±0.01µs 0.00%
precise_decimal::div/0 273.5±0.07ns 263.8±0.07ns -3.55%
precise_decimal::from_string/0 201.7±0.38ns 195.9±0.18ns -2.88%
precise_decimal::mul/0 284.8±0.13ns 282.1±0.09ns -0.95%
precise_decimal::mul/rust-native 262.3±0.21ns 263.7±0.17ns +0.53%
precise_decimal::mul/wasmer 4.0±0.00µs 4.1±0.00µs +2.50%
precise_decimal::mul/wasmer-call-native 816.9±0.16ns 824.9±0.65ns +0.98%
precise_decimal::mul/wasmi 72.5±0.06µs 73.6±0.03µs +1.52%
precise_decimal::mul/wasmi-call-native 3.9±0.01µs 4.0±0.01µs +2.56%
precise_decimal::pow/0 1574.5±0.34ns 1566.2±0.37ns -0.53%
precise_decimal::pow/rust-native 1279.4±1.12ns 1278.9±0.31ns -0.04%
precise_decimal::pow/wasmer 18.9±0.01µs 18.9±0.01µs 0.00%
precise_decimal::pow/wasmer-call-native 2.0±0.00µs 2.0±0.00µs 0.00%
precise_decimal::pow/wasmi 351.0±0.34µs 358.0±0.15µs +1.99%
precise_decimal::pow/wasmi-call-native 7.5±0.02µs 7.6±0.03µs +1.33%
precise_decimal::root/0 60.9±0.02µs 61.3±0.03µs +0.66%
precise_decimal::sub/0 8.3±0.00ns 8.3±0.01ns 0.00%
precise_decimal::to_string/0 742.6±0.47ns 744.8±0.24ns +0.30%
schema::validate_payload 333.1±0.20µs 331.9±0.25µs -0.36%
transaction::radiswap 5.2±0.04ms 5.2±0.03ms 0.00%
transaction::transfer 1685.8±2.67µs 1682.9±2.77µs -0.17%
transaction_processing::prepare 2.4±0.00ms 2.4±0.00ms 0.00%
transaction_processing::prepare_and_decompile 6.1±0.01ms 6.1±0.01ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 23.5±0.15ms 22.9±0.15ms -2.55%
transaction_validation::validate_manifest 43.7±0.03µs 43.8±0.54µs +0.23%
transaction_validation::verify_ecdsa 77.9±0.06µs 77.8±0.05µs -0.13%
transaction_validation::verify_ed25519 51.6±0.80µs 51.4±0.05µs -0.39%

@talekhinezh talekhinezh merged commit 7acc215 into develop Jan 8, 2024
25 checks passed
get_sbor_attribute_string_value(&attrs, "type_name")?.unwrap_or(ident.to_string());

let type_id = quote! {
::sbor::RustTypeId::novel_with_code(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for the future - we can consider just using core::any::TypeId::of::<Self> here, if we add where Self: 'static to the Describe trait (which should be fine) rust-lang/rust#72488

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is scary!

Type ids have some edges that this stabilization exposes to more contexts. It's possible for type ids to collide (but this is a bug). Since they can change between compiler versions, it's never valid to cast a type id to its underlying value.

And looks like it was actually un-stabilized as const - so perhaps we're good to wait for it to be improved... see discussion on rust-lang/rust#75923

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.

2 participants