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

Add scrypto dev container target #2004

Merged
merged 6 commits into from
Dec 3, 2024

Conversation

azizi-a
Copy link
Contributor

@azizi-a azizi-a commented Nov 15, 2024

Summary

  • Add new target in Docker file: scrypto-dev-container
  • Define old Docker file target: scrypto-builder

@azizi-a azizi-a changed the base branch from main to develop November 15, 2024 15:39
@azizi-a azizi-a force-pushed the add-scrypto-dev-container branch from 8ff2571 to 5f14446 Compare November 15, 2024 15:53
Copy link

github-actions bot commented Nov 15, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:36b7b31bc9

@azizi-a azizi-a force-pushed the add-scrypto-dev-container branch from 5f14446 to 3276dac Compare November 15, 2024 16:06
Copy link

github-actions bot commented Nov 15, 2024

Benchmark for 36b7b31

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 45.0±0.06ms 44.6±0.08ms -0.89%
costing::decode_encoded_i8_array_to_manifest_raw_value 19.5±0.05ms 19.3±0.01ms -1.03%
costing::decode_encoded_i8_array_to_manifest_value 41.8±0.07ms 42.5±0.10ms +1.67%
costing::decode_encoded_tuple_array_to_manifest_raw_value 72.1±0.13ms 72.2±0.09ms +0.14%
costing::decode_encoded_tuple_array_to_manifest_value 97.9±0.23ms 99.0±0.23ms +1.12%
costing::decode_encoded_u8_array_to_manifest_raw_value 32.7±0.08µs 32.5±0.25µs -0.61%
costing::decode_encoded_u8_array_to_manifest_value 41.8±0.09ms 42.4±0.12ms +1.44%
costing::decode_rpd_to_manifest_raw_value 14.6±0.03µs 14.5±0.02µs -0.68%
costing::decode_rpd_to_manifest_value 11.0±0.04µs 10.9±0.07µs -0.91%
costing::deserialize_wasm 1227.5±3.69µs 1282.5±2.89µs +4.48%
costing::execute_transaction_creating_big_vec_substates 697.4±4.90ms 711.4±10.69ms +2.01%
costing::execute_transaction_reading_big_vec_substates 603.2±1.19ms 593.3±1.79ms -1.64%
costing::instantiate_flash_loan 873.3±360.19µs 1052.0±1272.31µs +20.46%
costing::instantiate_radiswap 1019.7±1034.08µs 959.2±978.89µs -5.93%
costing::scrypto_malloc 656.3±1.13ms 659.7±1.84ms +0.52%
costing::scrypto_sbor_decode 652.4±1.12ms 653.3±1.25ms +0.14%
costing::scrypto_sha256 572.8±0.85ms 574.6±0.55ms +0.31%
costing::spin_loop_v1 514.4±3.17ms 512.2±1.00ms -0.43%
costing::spin_loop_v2 596.6±0.82ms 598.8±0.55ms +0.37%
costing::validate_sbor_payload 30.0±0.07µs 30.5±0.05µs +1.67%
costing::validate_sbor_payload_bytes 256.9±0.79ns 251.1±0.87ns -2.26%
costing::validate_secp256k1 76.6±0.08µs 76.5±0.06µs -0.13%
costing::validate_wasm 33.4±0.03ms 33.5±0.03ms +0.30%
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/wasmi 310.5±2.02ns 311.5±1.86ns +0.32%
decimal::add/wasmi-call-native 2.9±0.02µs 2.9±0.02µs 0.00%
decimal::div/0 173.7±0.25ns 173.9±0.27ns +0.12%
decimal::from_string/0 166.4±0.18ns 166.5±0.20ns +0.06%
decimal::mul/0 126.6±0.10ns 126.8±0.17ns +0.16%
decimal::mul/rust-native 124.8±0.04ns 128.5±0.06ns +2.96%
decimal::mul/wasmi 20.2±0.43µs 19.3±0.03µs -4.46%
decimal::mul/wasmi-call-native 3.1±0.02µs 3.1±0.02µs 0.00%
decimal::pow/0 614.0±0.20ns 592.4±0.19ns -3.52%
decimal::pow/rust-native 591.2±0.45ns 592.5±0.41ns +0.22%
decimal::pow/wasmi 94.6±0.42µs 91.5±0.13µs -3.28%
decimal::pow/wasmi-call-native 4.9±0.01µs 4.7±0.02µs -4.08%
decimal::root/0 8.3±0.01µs 8.1±0.01µs -2.41%
decimal::sub/0 8.2±0.00ns 8.4±0.01ns +2.44%
decimal::to_string/0 439.8±1.25ns 440.7±0.51ns +0.20%
large_transaction_processing::prepare 2.6±0.00ms 2.5±0.00ms -3.85%
large_transaction_processing::prepare_and_decompile 6.1±0.02ms 6.2±0.01ms +1.64%
large_transaction_processing::prepare_and_decompile_and_recompile 31.2±0.05ms 24.5±0.12ms -21.47%
metadata_validation::validate_urls 5.2±0.01µs 5.2±0.04µs 0.00%
precise_decimal::add/0 8.7±0.05ns 8.7±0.04ns 0.00%
precise_decimal::add/rust-native 10.7±0.01ns 10.8±0.11ns +0.93%
precise_decimal::add/wasmi 416.5±2.55ns 420.3±2.07ns +0.91%
precise_decimal::add/wasmi-call-native 4.0±0.02µs 3.9±0.01µs -2.50%
precise_decimal::div/0 289.6±0.42ns 289.8±0.55ns +0.07%
precise_decimal::from_string/0 201.0±0.35ns 201.3±0.20ns +0.15%
precise_decimal::mul/0 331.1±0.20ns 331.1±0.33ns 0.00%
precise_decimal::mul/rust-native 305.2±1.67ns 301.5±1.66ns -1.21%
precise_decimal::mul/wasmi 48.3±1.05µs 47.5±0.17µs -1.66%
precise_decimal::mul/wasmi-call-native 4.2±0.01µs 4.3±0.02µs +2.38%
precise_decimal::pow/0 1726.6±2.68ns 1734.5±1.43ns +0.46%
precise_decimal::pow/rust-native 1360.3±2.41ns 1356.0±0.93ns -0.32%
precise_decimal::pow/wasmi 234.6±4.28µs 229.1±0.94µs -2.34%
precise_decimal::pow/wasmi-call-native 7.4±0.02µs 7.3±0.05µs -1.35%
precise_decimal::root/0 58.7±0.02µs 57.8±0.03µs -1.53%
precise_decimal::sub/0 9.0±0.04ns 9.0±0.04ns 0.00%
precise_decimal::to_string/0 733.3±3.54ns 740.7±2.08ns +1.01%
schema::validate_payload 386.2±0.70µs 386.4±0.45µs +0.05%
transaction::radiswap 5.0±0.04ms 5.0±0.02ms 0.00%
transaction::transfer 1812.7±3.54µs 1803.0±1.88µs -0.54%
transaction_validation::validate_manifest 43.0±0.03µs 43.1±0.12µs +0.23%
transaction_validation::verify_bls_2KB 964.6±8.45µs 965.4±13.23µs +0.08%
transaction_validation::verify_bls_32B 989.8±11.78µs 976.8±17.05µs -1.31%
transaction_validation::verify_ecdsa 74.4±0.06µs 74.4±0.11µs 0.00%
transaction_validation::verify_ed25519 42.3±0.10µs 42.3±0.06µs 0.00%

@azizi-a azizi-a marked this pull request as ready for review November 18, 2024 08:53
@azizi-a azizi-a requested a review from dhedey November 21, 2024 11:02
Copy link
Contributor

@dhedey dhedey left a comment

Choose a reason for hiding this comment

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

Really nice!

Out of interest:

  • How is this supposed to be used?
    • Do people add some kind of devcontainer file to their repo?
  • How can this be tested locally during development?
    • How have you been testing it?
    • Might we want some script to build the dev container locally, possibly commented in the Dockerfile to help future people to tweak it?
  • Perhaps we need something in CI to test the building of this target? Like the dev-container equivalent of ci-scrypto-builder.yml and publish-scrypto-builder.yml

Dockerfile Outdated
@@ -50,6 +50,13 @@ WORKDIR /app

RUN cargo install --path ./radix-clis

FROM base-image as dev-container
RUN apt install -y bash-completion powerline
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth a quick comment explaining that this installs a better prompt for a better dev experience (with maybe a link to the powerline website?)

Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@azizi-a
Copy link
Contributor Author

azizi-a commented Nov 21, 2024

  • How is this supposed to be used?

    • Do people add some kind of devcontainer file to their repo?

My intention is to create another repo with the Hello template in and that uses this image as it's dev container

  • How can this be tested locally during development?

    • How have you been testing it?

Build the image with docker build . --target=dev-container -t scrypto-dev-container then opened a new VS Code instance with .devcontainer/devcontainer.json containing the following.

{
  "name": "Radix Scrypto",
  "image": "scrypto-dev-container",
  "customizations": {
    "vscode": {
      "extensions": [
        "rust-lang.rust-analyzer",
        "tamasfe.even-better-toml",
        "radixpublishing.radix-developer-tools"
      ]
    }
  } 
}

That would be the same for the new repo also containing a Hello template

  • Might we want some script to build the dev container locally, possibly commented in the Dockerfile to help future people to tweak it?

Will add this

  • Perhaps we need something in CI to test the building of this target? Like the dev-container equivalent of ci-scrypto-builder.yml and publish-scrypto-builder.yml

Will add this too

@azizi-a azizi-a force-pushed the add-scrypto-dev-container branch from cda17ea to 44da993 Compare November 22, 2024 14:13
@azizi-a azizi-a requested a review from dhedey November 22, 2024 14:14
@azizi-a azizi-a changed the base branch from develop to release/cuttlefish November 27, 2024 18:02
@azizi-a azizi-a force-pushed the add-scrypto-dev-container branch from 44da993 to 5aebfa9 Compare November 29, 2024 15:11
@azizi-a azizi-a force-pushed the add-scrypto-dev-container branch from 5aebfa9 to cb560a5 Compare December 3, 2024 09:44
Dockerfile Show resolved Hide resolved
@azizi-a azizi-a merged commit dec63d0 into release/cuttlefish Dec 3, 2024
31 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