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

feat: Add in place update support to versioned macro #1755

Merged
merged 21 commits into from
Apr 24, 2024

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Mar 30, 2024

Summary

Pulls in (and productionizes) versioned macro changes from #1744 ... and adds a lot of other stuff.

Key changes:

  • Updates to Versioned macros - any external define_versioned! calls need updating.
  • Updates to the native blueprint state macros to support kind: StaticMultiVersioned
  • Updates to SBOR derive to support as_type and ignored enum variants, and tweaks to when categorize_types is needed to be more intuitive.
  • Added a PermitSborAttributes derive which simply allows #[sbor] attributes to be used even if the type doesn't derive Sbor. This means a helper macro can add sbor-specific attributes, but leave it up to the user to decide if they actually need Sbor on the type or not.
  • A new eager_replace! procedural macro which is a pre-processor style macro intended for use in declarative macros. It is effectively a more powerful version than paste! which is also inspired by quote! / parse_quote! and gets around this issue: Generate doc-comments based on provided tokens dtolnay/paste#40 (comment) - and generally solves a lot of frustrations I've had building out the more gnarly declarative macros in the code base - to see it in practice, see sbor-derive/lib.rs and sbor/versioned.rs.

Details

Versioned

  • Most of the methods on the Versioned_ types have been renamed to be much clearer.
  • Versioned types now contain an internal _Versions enum. See yak shaving below.
  • Added a fully_update_to_latest_version_mut method to Versioned enums, which takes a Versioned enum, updates it in place if necessary, and returns a &mut to the resultant latest content. This makes for much more streamlined updates of nested state, which was why I added it when doing prototyping in [ON HOLD] declare_native_blueprint_state extensions #1744.
  • Also added methods for e.g. as_unique_version_mut to ease use of single versioned item
  • Improve rust docs on the versioned enum interface
  • A few other drive-by fixes of compiler warnings

Yak-shaving background

  • To support in-place update of the Versioned enum from a &mut reference, we need some temporary state to store into the enum whilst we take out the current value and try to upgrade it (which requires taking ownership of the value, because the upgrade mechanism uses the more flexible From mechanism which needs an owned object).
  • After exploring adding a fake enum variant, we ended up deciding to explore having a wrapper struct, and wrapping the versions into an Option so that we could temporarily replace it with None during the upgrade process.
  • But adding that attribute caused Versioned types which didn't have any #[derive(Sbor)] annotations to get a compile error (about an unrecognized sbor attribute), so I also added a PermitSborAttributes derive macro which doesn't actually derive anything, but does enable #[sbor(..)] annotations to be present without giving a confusing compile error, to support versioned types which don't use SBOR derives

Testing

Added tests for the SBOR changes to sbor-tests.

The versioned changes are tested in the rust doc tests, and by virtue of being used by other tests.

Update Recommendations

For Internal Integrators

Anyone consuming a versioned type may need to change from using a match to using one of the many new methods, e.g.:

  • fully_update_into_latest_version (self -> Self::Latest)
  • fully_update_to_latest_version_mut (&mut self -> &mut Self::Latest)
  • as_unique_version_ref (&self -> &Self::Latest) - only on single versioned enums

If you need to match the versions, you can call .as_versions_ref() / .as_versions_mut() / .into_versions() and then match on that.

Anyone creating a versioned type will need to update the declaration to e.g. something like the following. Notable changes are 1. removal of the enum type, and addition of the Versions type.

define_single_versioned!(
    #[derive(Debug, Clone, PartialEq, Eq, Sbor)]
    #[sbor(child_types = "S::CustomTypeKind<LocalTypeId>, S::CustomTypeValidation")]
    pub VersionedSchema(SchemaVersions)<S: CustomSchema> => Schema<S> = SchemaV1::<S>
);
define_versioned!(
    #[derive(Debug, Clone, PartialEq, Eq, Sbor)]
    VersionedExample(ExampleVersions) {
        previous_versions: [
            1 => ExampleV1: { updates_to: 2 },
            2 => ExampleV2: { updates_to: 4 },
            3 => ExampleV3: { updates_to: 4 },
        ],
        latest_version: {
            4 => Example = ExampleV4,
        },
    }
);

Copy link

github-actions bot commented Mar 30, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:67bdf40976

Copy link

github-actions bot commented Mar 30, 2024

Benchmark for 67bdf40

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 65.8±0.16ms 65.0±0.14ms -1.22%
costing::decode_sbor 11.2±0.03µs 10.7±0.01µs -4.46%
costing::decode_sbor_bytes 29.2±0.16µs 29.6±0.06µs +1.37%
costing::deserialize_wasm 1292.1±8.17µs 1279.4±2.67µs -0.98%
costing::instantiate_flash_loan 3.7±0.33ms 3.8±0.46ms +2.70%
costing::instantiate_radiswap 5.8±0.08ms 5.7±0.05ms -1.72%
costing::spin_loop 21.4±0.04ms 22.0±0.13ms +2.80%
costing::validate_sbor_payload 30.6±0.12µs 27.3±0.06µs -10.78%
costing::validate_sbor_payload_bytes 244.0±0.90ns 236.3±0.57ns -3.16%
costing::validate_secp256k1 76.3±0.08µs 76.2±0.14µs -0.13%
costing::validate_wasm 36.7±0.07ms 36.7±0.03ms 0.00%
decimal::add/0 8.4±0.00ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.8±0.00ns 9.8±0.00ns 0.00%
decimal::add/wasmer 113.4±0.08ns 114.0±0.15ns +0.53%
decimal::add/wasmer-call-native 458.1±0.43ns 447.9±0.41ns -2.23%
decimal::add/wasmi 604.4±1.52ns 576.3±2.14ns -4.65%
decimal::add/wasmi-call-native 5.4±0.01µs 5.1±0.02µs -5.56%
decimal::div/0 189.9±0.06ns 191.8±0.32ns +1.00%
decimal::from_string/0 155.2±0.43ns 152.0±0.22ns -2.06%
decimal::mul/0 141.9±0.08ns 142.3±0.22ns +0.28%
decimal::mul/rust-native 137.4±0.55ns 137.7±0.13ns +0.22%
decimal::mul/wasmer 1533.8±1.28ns 1525.2±1.13ns -0.56%
decimal::mul/wasmer-call-native 586.6±0.44ns 585.2±0.39ns -0.24%
decimal::mul/wasmi 41.5±0.06µs 40.9±0.15µs -1.45%
decimal::mul/wasmi-call-native 5.5±0.01µs 5.5±0.01µs 0.00%
decimal::pow/0 651.1±0.53ns 654.1±0.47ns +0.46%
decimal::pow/rust-native 635.6±0.34ns 638.5±1.55ns +0.46%
decimal::pow/wasmer 6.6±0.01µs 6.5±0.01µs -1.52%
decimal::pow/wasmer-call-native 1041.8±0.42ns 1047.1±0.58ns +0.51%
decimal::pow/wasmi 197.3±0.25µs 192.0±0.69µs -2.69%
decimal::pow/wasmi-call-native 5.5±0.02µs 5.2±0.01µs -5.45%
decimal::root/0 7.8±0.01µs 8.1±0.01µs +3.85%
decimal::sub/0 8.5±0.01ns 8.5±0.01ns 0.00%
decimal::to_string/0 437.9±0.50ns 446.4±0.97ns +1.94%
precise_decimal::add/0 10.1±0.01ns 9.4±0.13ns -6.93%
precise_decimal::add/rust-native 11.4±0.00ns 11.4±0.00ns 0.00%
precise_decimal::add/wasmer 124.0±0.21ns 124.2±0.21ns +0.16%
precise_decimal::add/wasmer-call-native 487.9±0.22ns 504.6±1.43ns +3.42%
precise_decimal::add/wasmi 780.4±1.62ns 737.1±1.09ns -5.55%
precise_decimal::add/wasmi-call-native 6.9±0.01µs 6.7±0.03µs -2.90%
precise_decimal::div/0 304.4±0.17ns 303.5±0.07ns -0.30%
precise_decimal::from_string/0 197.8±0.20ns 197.8±0.27ns 0.00%
precise_decimal::mul/0 343.0±0.57ns 346.2±0.40ns +0.93%
precise_decimal::mul/rust-native 305.6±0.82ns 305.0±0.86ns -0.20%
precise_decimal::mul/wasmer 3.4±0.00µs 3.5±0.00µs +2.94%
precise_decimal::mul/wasmer-call-native 817.9±1.55ns 822.6±8.82ns +0.57%
precise_decimal::mul/wasmi 106.4±0.28µs 104.9±0.20µs -1.41%
precise_decimal::mul/wasmi-call-native 7.4±0.02µs 7.1±0.04µs -4.05%
precise_decimal::pow/0 1855.7±5.10ns 1853.4±2.91ns -0.12%
precise_decimal::pow/rust-native 1489.8±2.12ns 1464.2±4.46ns -1.72%
precise_decimal::pow/wasmer 16.2±0.01µs 16.1±0.05µs -0.62%
precise_decimal::pow/wasmer-call-native 2.1±0.00µs 2.1±0.00µs 0.00%
precise_decimal::pow/wasmi 511.4±0.72µs 504.9±1.16µs -1.27%
precise_decimal::pow/wasmi-call-native 13.2±0.02µs 12.8±0.02µs -3.03%
precise_decimal::root/0 56.6±0.08µs 56.5±0.01µs -0.18%
precise_decimal::sub/0 9.5±0.01ns 9.6±0.06ns +1.05%
precise_decimal::to_string/0 743.0±7.28ns 718.3±2.14ns -3.32%
schema::validate_payload 356.3±0.71µs 343.9±0.33µs -3.48%
transaction::radiswap 5.3±0.02ms 5.3±0.03ms 0.00%
transaction::transfer 1762.9±18.01µs 1752.8±13.35µs -0.57%
transaction_processing::prepare 2.3±0.01ms 2.2±0.00ms -4.35%
transaction_processing::prepare_and_decompile 6.1±0.03ms 6.0±0.04ms -1.64%
transaction_processing::prepare_and_decompile_and_recompile 24.3±0.73ms 24.2±0.73ms -0.41%
transaction_validation::validate_manifest 42.2±0.06µs 42.2±0.21µs 0.00%
transaction_validation::verify_bls_2KB 1049.3±71.25µs 987.8±14.92µs -5.86%
transaction_validation::verify_bls_32B 1083.0±125.36µs 1012.5±33.89µs -6.51%
transaction_validation::verify_ecdsa 74.4±0.76µs 74.1±0.03µs -0.40%
transaction_validation::verify_ed25519 55.2±0.48µs 55.1±0.04µs -0.18%

radix-engine/src/vm/wasm/wasmi.rs Outdated Show resolved Hide resolved
assets/blueprints/radiswap/src/lib.rs Outdated Show resolved Hide resolved
@dhedey dhedey marked this pull request as ready for review March 30, 2024 10:15
Copy link
Member

@iamyulong iamyulong left a comment

Choose a reason for hiding this comment

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

As discussed, there is concern about the update-in-progress enum variant. We will need to address this before merging.

@iamyulong
Copy link
Member

Looks good.

One idea to explore is to make the attributes more structural, such as

#[sbor(as_type("type", as_ref="", from_value="", transparent_name))]

@dhedey dhedey merged commit 776cfea into develop Apr 24, 2024
28 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.

2 participants