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

Experimental RuntimeDebug -> Debug substitutions. #3107

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hirschenberger
Copy link
Contributor

@hirschenberger hirschenberger commented Jan 29, 2024

see #3005

My local results are:

╭───┬────────────────────────────────────────────────────────────────────────────────╮
│ 0 │ fast_runtime_binary.rs.compact.compressed.wasm:   NewSize: 1493084 (+1425)     │
│ 1 │ rococo_runtime.compact.compressed.wasm:           NewSize: 1493143 (+793)      │
│ 2 │ westend_runtime.compact.compressed.wasm:          NewSize: 1626913 (+1973)     │
╰───┴────────────────────────────────────────────────────────────────────────────────╯

compiled with cargo build --profile=production

EDIT: I removed the kitchensink as it was a relic from previous runs

  • Replace all derives RuntimeDebug and RuntimeDebugNoBound to Debug
  • Remove all use expressions for RuntimeDebug and RuntimeDebugNoBound
  • Deprecate RuntimeDebug and RuntimeDebugNoBound
  • Remove all RuntimeDebug and RuntimeDebugNoBound references on docs and doctests
  • Rerun the before-and-after size comparison of wasm runtimes

@ggwpez
Copy link
Member

ggwpez commented Jan 29, 2024

I think we can also deprecate the RuntimeDebug derive macro then or?

@bkchr
Copy link
Member

bkchr commented Jan 29, 2024

I think we can also deprecate the RuntimeDebug derive macro then or?

Yes.

@ggwpez
Copy link
Member

ggwpez commented Feb 8, 2024

@hirschenberger I think we can go ahead with this?
Its also fine if you dont want to do the deprecation, we can do it in another MR.

@hirschenberger
Copy link
Contributor Author

@hirschenberger I think we can go ahead with this? Its also fine if you dont want to do the deprecation, we can do it in another MR.

Ok, so I'll continue.

The next steps would be to deprecate RuntimeDebug and fix all resulting deprecation warnings, correct?

@bkchr
Copy link
Member

bkchr commented Mar 11, 2024

The next steps would be to deprecate RuntimeDebug and fix all resulting deprecation warnings, correct?

Yes!

@kianenigma kianenigma added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Mar 21, 2024
@bkchr
Copy link
Member

bkchr commented Apr 23, 2024

@hirschenberger are you interested on continue working on this?

@hirschenberger
Copy link
Contributor Author

@hirschenberger are you interested on continue working on this?

Yes, just got a new notebook which makes development on that issue much snappier.

@bkchr
Copy link
Member

bkchr commented Apr 24, 2024

Ty! I'm quite interesting in this and would be happy to merge this in the near future.

@hirschenberger
Copy link
Contributor Author

Ty! I'm quite interesting in this and would be happy to merge this in the near future.

I try to deliver ASAP

@hirschenberger
Copy link
Contributor Author

@bkchr What about RuntimeDebugNoBound? Also deprecate and substitute?

@bkchr
Copy link
Member

bkchr commented Apr 24, 2024

@bkchr What about RuntimeDebugNoBound? Also deprecate and substitute?

Yes!

@hirschenberger
Copy link
Contributor Author

@bkchr Hey Basti, after subsituting all RtDebug und RtDebugNoBound I get a lot of errors like:

error[E0277]: `A` doesn't implement `std::fmt::Debug`
   --> substrate/frame/support/src/traits/tokens/fungible/mod.rs:204:25
    |
204 |     > Consideration<A> for FreezeConsideration<A, F, R, D>
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `A` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`

So there are many generic type parameters missing a Debug bound. Should I extend these all?

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6085157

@bkchr
Copy link
Member

bkchr commented Apr 29, 2024

RtDebugNoBound

You can not just replace this with Debug, it should be replaced by DebugNoBound to not require bounds for the generic types.

@hirschenberger
Copy link
Contributor Author

Still working on this, I tried to build my own refactoring tool using syn and it works ok, but there are some nasty edge-cases.

I also found the nice ast-grep tool which I'm giving a quick try.

@bkchr
Copy link
Member

bkchr commented Jul 17, 2024

@hirschenberger ping.

@ggwpez ggwpez mentioned this pull request Sep 10, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants