Skip to content

Reduce unnecessary dependencies #32

Closed
#86
Closed
Reduce unnecessary dependencies#32
#86
Labels
enhancementNew feature or requestgood first issueGood for newcomers

Description

@liamaharon
liamaharon
opened on Aug 30, 2023

A significant amount of compilation time is spent on stuff like the kitchensink runtime, which clearly is unnecessary for try-runtime-cli.

Would be great if someone could

  1. Audit the compilation time and flag unnecessary dependencies that're dragged in
  2. Investigate if we can shuffle dependencies around and/or make changes upstream that would allow us to grab only what we need without unnecessary bloat

Activity

  1. liamaharon
    added
    enhancementNew feature or request
    good first issueGood for newcomers
    on Aug 30, 2023

  2. eagr commented on Dec 2, 2023

    @eagr
    eagr
    on Dec 2, 2023
    Contributor
    image

  3. liamaharon commented on Dec 2, 2023

    @liamaharon
    liamaharon
    on Dec 2, 2023
    ContributorAuthor

    We should be able to remove the kitchensink dep I think by creating a bare runtime only exposing the runtime apis we want to use.

  4. eagr commented on Dec 3, 2023

    @eagr
    eagr
    on Dec 3, 2023
    Contributor
    cargo tree --invert --locked --package kitchensink-runtime

kitchensink-runtime v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
├── staging-node-cli v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│   └── substrate-cli-test-utils v0.1.0 (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│       [dev-dependencies]
│       └── try-runtime-core v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/core)
│           └── try-runtime-cli v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/cli)
└── staging-node-executor v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
    ├── staging-node-cli v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48) (*)
    └── try-runtime-cli v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/cli)
    [dev-dependencies]
    └── try-runtime-core v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/core) (*)

    Yeah, we need to substitute substrate-cli-test-utils and staging-node-executor, to be specific.

    This ain't trivial. The most viable path I could see is to identify and flag the optional dependencies upstream, and feature gate them all the way up.

  5. eagr commented on Dec 3, 2023

    @eagr
    eagr
    on Dec 3, 2023
    Contributor

    And kitchensink-runtime may not even be the culprit.

    cargo tree --invert --locked --package wasm-opt-sys

wasm-opt-sys v0.116.0
├── wasm-opt v0.116.0
│   └── substrate-wasm-builder v5.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│       [build-dependencies]
│       └── kitchensink-runtime v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│           ├── staging-node-cli v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│           │   └── substrate-cli-test-utils v0.1.0 (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│           │       [dev-dependencies]
│           │       └── try-runtime-core v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/core)
│           │           └── try-runtime-cli v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/cli)
│           └── staging-node-executor v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48)
│               ├── staging-node-cli v3.0.0-dev (https://github.com/paritytech/polkadot-sdk?branch=master#1d5d4a48) (*)
│               └── try-runtime-cli v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/cli)
│               [dev-dependencies]
│               └── try-runtime-core v0.5.1 (/Users/eagr/Projects/eagr/fork/substrate-try-runtime-cli/core) (*)
└── wasm-opt-cxx-sys v0.116.0
    └── wasm-opt v0.116.0 (*)

  6. liamaharon commented on Dec 3, 2023

    @liamaharon
    liamaharon
    on Dec 3, 2023
    ContributorAuthor

    This ain't trivial. The most viable path I could see is to identify and flag the optional dependencies upstream, and feature gate them all the way up.

    I think there is a path to just removing the need for these deps entirely, rather than messing with their deps to make them lighter weight.

  7. eagr commented on Dec 3, 2023

    @eagr
    eagr
    on Dec 3, 2023
    Contributor

    You mean to pull the necessary source code into this repo right? I'm worried that this might get out of hand in some way, but maybe not as bad as I think. Let me test it out.

  8. liamaharon commented on Dec 3, 2023

    @liamaharon
    liamaharon
    on Dec 3, 2023
    ContributorAuthor

    Not pull it out 1:1, but see what it is being used for, e.g. getting HostFunctions, and write our own minimal HostFunctions exposing only what we need instead of pulling in an entire runtime filled with crates just for the HostFunctions.

  9. eagr commented on Dec 3, 2023

    @eagr
    eagr
    on Dec 3, 2023
    Contributor

    Tried substituting substrate-cli-test-utils. Giving up, too much to untangle in order to carve out kitchensink-runtime. It still seems to me that it's better to be fixed upstream by refactoring and feature-gating. Not entirely sure if this is related, I heard you're planning to replace wasmtime with polkavm. If that's the case, there would probably be a huge rewrite? I guess the problem should be flagged if not already.

    And test code may be alright. It feels like it would become a maintenance burden (need to keep up with the sdk) or even backfire in the long run, going this route for production code.

  10. liamaharon
    closed this as completedin #86on May 26, 2024
  11. liamaharon
    added a commit that references this issue on May 26, 2024

    Inherents and empty block production overhaul (#86)

    251438f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestgood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions