Improve debuggability in wasm

[bkchr]

Over the years we have tried to prevent any kind of leakage of the Debug trait into wasm to decrease the size of the wasm binary. However, AFAIK we never really looked into the impact of having Debug implemented for types. Especially as the linker should remove unused Debug implementation any way. We even have introduced RuntimeDebug that just prints wasm:stripped on no_std. The problem with all this is that it complicates debugging in wasm. In the future when there will be no native runtime, it is even more important. So, we should look into deriving Debug everywhere instead of RuntimeDebug. We should also change RuntimeDebug to always derive the default Debug to not break any downstream code, but still have proper debug output.

So, for this issue:

1. Compile some runtimes and note their size.
2. Change RuntimeDebug to always derive default Debug.
3. Compile again and compare the runtime sizes against the ones noted in 1.
4. If the difference in size isn't that big, remove RuntimeDebug everywhere in the code base.

[hirschenberger]

I can do this...

[athei]

We should also change RuntimeDebug to always derive the default Debug to not break any downstream code, but still have proper debug output.

Doing just this for this experiment would probably much easier :) Of course if we decide to merge we probably want to replace the deprecated RuntimeDebug uses.

I can do this...

Would be appreciated. Remember to build with --profile production so that LTO is applied. Don't try to build the client though as this will take a long time with lto.

We also have to keep in mind that there are a bunch of "manual" RuntimeDebug left in the code base. Those might not trivially be replaceable because they use floats or other things we don't want to use in our runtimes. But we can do this in follow ups I guess. Example:

polkadot-sdk/substrate/primitives/arithmetic/src/fixed_point.rs

Lines 902 to 923 in a78ff7d

	impl sp_std::fmt::Debug for $name {
	#[cfg(feature = "std")]
	fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
	let integral = {
	let int = self.0 / Self::accuracy();
	let signum_for_zero = if int == 0 && self.is_negative() { "-" } else { "" };
	format!("{}{}", signum_for_zero, int)
	};
	let precision = (Self::accuracy() as f64).log10() as usize;
	let fractional = format!(
	"{:0>weight$}",
	((self.0 % Self::accuracy()) as i128).abs(),
	weight = precision
	);
	write!(f, "{}({}.{})", stringify!($name), integral, fractional)
	}
	#[cfg(not(feature = "std"))]
	fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
	Ok(())
	}
	}

In addition to float it also uses format! which is not available in sp_std. One would have to manually import it from alloc which is possible but frowned upon in this code base. That said, in this example using format! is not really necessary and all arguments could just be inlined into one big write!.

[bkchr]

Doing just this for this experiment would probably much easier :) Of course if we decide to merge we probably want to replace the deprecated RuntimeDebug uses.

If you have read my steps, I have said exactly this :P

[athei]

Okay then I said nothing 🤝

[hirschenberger]

I did some initial tests.

I scripted some RuntimeDebug -> Debug substitutions in derive attributes. The diff shows 462 replacements.

Then I compiled with these changes and compared the wasm binary sizes:

╭───┬───────────────────────────────────────────────────────────────╮
│ 0 │ kitchensink_runtime.wasm:    NewSize: 8852.6 KB (+4.5 KiB)    │
│ 1 │ rococo_runtime.wasm:         NewSize: 6884.3 KB (+6.7 KiB)    │
│ 2 │ westend_runtime.wasm:        NewSize: 7150.2 KB (+8.6 KiB)    │
╰───┴───────────────────────────────────────────────────────────────╯

So it seems there's nearly no difference in size.

[ggwpez]

Did you compile with --profile production? The WASM blobs seem quite large. Normally its about 3MiB uncompressed i think.
But anyway the difference seems quite small.

[athei]

Those are probably the uncompressed artifacts. Can you also add the *.compact.compressed.wasm sizes? I think those are more interesting since our limits are on the compressed size? Not sure about this.

[hirschenberger]

Here are the results for the compressed runtimes. I'll double-check it on monday.

╭───┬─────────────────────────────────────────────────────────────────────────────────╮
│ 0 │ kitchensink_runtime.compact.compressed.wasm:        NewSize: 4250.6 KB (+0 B)   │
│ 1 │ rococo_runtime.compact.compressed.wasm:             NewSize: 1492.3 KB (+0 B)   │
│ 2 │ westend_runtime.compact.compressed.wasm:            NewSize: 1624.9 KB (+0 B)   │
╰───┴─────────────────────────────────────────────────────────────────────────────────╯

[bkchr]

Yes our limits are on the compressed file size! Looks good :)

[athei]

But zero size gain is unexpected, isn't it? Since those debug implementations are actually used to print log lines I would expect the binary to get bigger. Or am I missing something?

[hirschenberger]

I must recalculate the gains with byte-precision to get the exact numbers. I used 0.1KB precision for the above calculation so maybe it's <100bytes or I made an error.
So I doublecheck my numbers on monday.

[hirschenberger]

Here are the numbers in bytes:

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

❯ git diff | grep "RuntimeDebug" | wc -l
462

[athei]

Just from a gut feeling it seems too good. I could be wrong but I think we need to make sure that no mistake happened.

Can you:

1. Open a PR with the changes you applied to the code base. Try not to find and replace everything in oder to keep the diff small. I think you can just alias/re-export RuntimeDebug to Debug.

2. Post the exact commands you used to build (you used --production, right?) and print the sizes in the PR description.

Then we can reasonably sure that no error happened.

[bkchr]

Just from a gut feeling it seems too good. I could be wrong but I think we need to make sure that no mistake happened.

I mean the main difference should be extra string being added to the runtimes. We also don't log everything etc. So, maybe not that wrong what we see there.

[athei]

Maybe. But it is not only strings. Also the code that does the formatting at runtime. True, the fmt infrastructure is already in the runtime.

We also don't log everything

That seems to be the most plausible explanation to me.

[hirschenberger]

@bkchr Sorry Bastian, I think I'll not have the time in the near future that this Issue requires. Feel free to reassign it.