improve benchmarking error output

[ropottnik]

fixes #7386

This PR adds pallet and benchmark to benchmark error output.

The PR also changes the wrapped type in the error variant of fn dispatch_benchmark from sp_std::RuntimeString to Vec<u8> and creates a dedicated function join_u8_sequences to achieve concatenation of character vectors.

I couldn't find an equivalent of format! in a non_std environment (would appreciate if anyone could point my nose to it if it is available). That's why I implemented the clumsy concatenation function.

My struggles with using RuntimeStrings led to me changing the function signatures to return Vec<u8>. I posted a question here in the substrate technical chat but to my knowledge got no reply. Again, I appreciate any suggestion on how to work with RuntimeStrings and therefore leave the signatures as they are.

Any feedback is welcome!

[gui1117]

is it doable to have a test somehow ?
maybe calling some internal function or with a ui test using trybuild (you can take a look at frame/support/test/tests/construct_runtie_ui.rs )

[ropottnik]

@thiolliere thanks for your suggestion. I wanted to do just that and discussed it with @shawntabrizi. I believe he was of the opinion that writing a test for it wasn't strictly necessary to consider the issues solved. However, I'm open for suggestions on how to do that.

Maybe there should be a new issue like "test benchmark reporting"? I'd be of course happy to take it up if I also know whom to ask. It was a bit hard to reach anyone for detailed questions and I also didn't get a reply when posting to the substrate technical chat...

[gui1117]

then I think it is fine without tests

[gui1117]

I feel like returning a Vec<u8> while it will be interpreted a String in the client is a bit confusing.

But I'm ok anyway, code looks good otherwise

[bkchr]

Why not use format!?

[ropottnik]

I think format! is not available in a no_std environment. Is it?

[bkchr]

It is ;) sp_std::alloc::format!, but 🤫

[ropottnik]

oh thanks. that would make things a lot easier.
so if I replace the join_u8_sequences(...) call with something like sp_std::alloc::format!("blabla {}", 1u32).as_bytes().to_vec(), recompiling fails with

   Compiling node-template-runtime v2.0.0 (/Users/simon/dev/substrate/bin/node-template/runtime)
error[E0433]: failed to resolve: could not find `format` in `alloc`
   --> bin/node-template/runtime/src/lib.rs:471:4
    |
471 |             add_benchmark!(params, batches, pallet_timestamp, Timestamp);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not find `format` in `alloc`
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: could not find `format` in `alloc`
   --> bin/node-template/runtime/src/lib.rs:470:4
    |
470 |             add_benchmark!(params, batches, pallet_balances, Balances);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not find `format` in `alloc`
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: could not find `format` in `alloc`
   --> bin/node-template/runtime/src/lib.rs:469:4
    |
469 |             add_benchmark!(params, batches, frame_system, SystemBench::<Runtime>);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not find `format` in `alloc`
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

what am I doing wrong?

[ropottnik]

I mean I see that without_std.rs does public extern crate alloc; and that format! is in there. but I can't seem to get it to work here.

[gui1117]

it is hard so say why without the code, did you use sp_std::alloc or just alloc ?

[bkchr]

Ahh that error is probably from the native build? For native we don't export the alloc crate.

[ropottnik]

ok. so format! is not available then?

[bkchr]

On native format! is provided by the rustc prelude, meaning it is "just" available.

So you need to use some cfgs around this call.

[ropottnik]

Aah, thanks @bkchr and @thiolliere for enlightening me. I wasn't aware of the fact that it was compiled once with and once without std features. Now it makes a lot more sense to me.

[ropottnik]

@thiolliere @bkchr I have now refactored the solution quite a bit to use RuntimeString and format!. Can you check again? Thanks!

[gui1117]

minor note: the runtime doesn't check that this is a string, but the cli does, so it's fine to me

[shawntabrizi]

Great. Thank you @ropottnik for the PR and @thiolliere and @bkchr for taking over mentorship :)

[ropottnik]

these test failures don't look related to what I did. should I rebase to let the CI/CD run again?

[gui1117]

@ropottnik can you merge substrate master in your branch ? I think the CI fails because of it

[gui1117]

bot merge

[ghost]

Trying merge.