search

gnzlbg / cargo-asm

cargo subcommand showing the assembly or llvm-ir generated for Rust code
https://github.com/gnzlbg/cargo-asm
Other
1.19k stars 37 forks source link

target() is called thousands of times #102

Closed therealprof closed 5 years ago

therealprof commented 5 years ago

I was looking at what would be needed to use the fantastic cargo-asm on embedded and (stupidly) instrumented target::target() with a println!() just to figure out that my console is completely flooded due to the many calls. I figure this cannot be good thing performance-wise. ;)

gnzlbg commented 5 years ago

Do you know where it is called from? Like 99% of the CPU time goes to rustc in the tests i've done, but you might be hitting a pathological case. A reproducer would be appreciated.

therealprof commented 5 years ago

Sure, first I get a ton of target() call before any output:

stack backtrace:
   0:        0x1070718ce - backtrace::backtrace::trace::h86791de082c35cc1
   1:        0x1070708ac - <backtrace::capture::Backtrace as core::default::Default>::default::h4469bf24ec414578
   2:        0x10704decf - cargo_asm::target::target::hfbb6a0b6b47ce549
   3:        0x107035bc3 - cargo_asm::asm::ast::File::new::h9980e51a47eef44d
   4:        0x10704b7c1 - cargo_asm::asm::parse::function::hfc7e87f364d45dd1
   5:        0x1070558c1 - cargo_asm::asm::run::hc4ed722868471097
   6:        0x10704f67f - cargo_asm::main::h516cbf768595538d
   7:        0x10704fb85 - std::rt::lang_start::{{closure}}::h8ff00e418f7890ff
   8:        0x1070ef347 - std::panicking::try::do_call::h20c95c457c762d5f
   9:        0x1070f680e - ___rust_maybe_catch_panic
  10:        0x1070efd6d - std::rt::lang_start_internal::he82d111f45e8971a
  11:        0x10704faf8 - _main

And then per printed instruction two more calls:

stack backtrace:
   0:        0x1070718ce - backtrace::backtrace::trace::h86791de082c35cc1
   1:        0x1070708ac - <backtrace::capture::Backtrace as core::default::Default>::default::h4469bf24ec414578
   2:        0x10704decf - cargo_asm::target::target::hfbb6a0b6b47ce549
   3:        0x1070381be - cargo_asm::asm::ast::Instruction::is_jump::h299e30eb46160d93
   4:        0x10703ce3e - cargo_asm::display::print::ha027ae6f9248ba63
   5:        0x1070567f3 - cargo_asm::asm::run::hc4ed722868471097
   6:        0x10704f67f - cargo_asm::main::h516cbf768595538d
   7:        0x10704fb85 - std::rt::lang_start::{{closure}}::h8ff00e418f7890ff
   8:        0x1070ef347 - std::panicking::try::do_call::h20c95c457c762d5f
   9:        0x1070f680e - ___rust_maybe_catch_panic
  10:        0x1070efd6d - std::rt::lang_start_internal::he82d111f45e8971a
  11:        0x10704faf8 - _main
stack backtrace:
   0:        0x1070718ce - backtrace::backtrace::trace::h86791de082c35cc1
   1:        0x1070708ac - <backtrace::capture::Backtrace as core::default::Default>::default::h4469bf24ec414578
   2:        0x10704decf - cargo_asm::target::target::hfbb6a0b6b47ce549
   3:        0x10703894e - cargo_asm::asm::ast::Instruction::is_call::h97fa205a2199f0d2
   4:        0x10703ce76 - cargo_asm::display::print::ha027ae6f9248ba63
   5:        0x1070567f3 - cargo_asm::asm::run::hc4ed722868471097
   6:        0x10704f67f - cargo_asm::main::h516cbf768595538d
   7:        0x10704fb85 - std::rt::lang_start::{{closure}}::h8ff00e418f7890ff
   8:        0x1070ef347 - std::panicking::try::do_call::h20c95c457c762d5f
   9:        0x1070f680e - ___rust_maybe_catch_panic
  10:        0x1070efd6d - std::rt::lang_start_internal::he82d111f45e8971a
  11:        0x10704faf8 - _main
therealprof commented 5 years ago

I'm not sure about the performance impact wrt to rustc calls but I also don't see a reason to construct a plethora of Strings with the same target information, only to drop it again a few cycles later. It's highly unlikely that the target will change between lines of the same function. 😅

gnzlbg commented 5 years ago

I suppose target could return a &'static str, or just use a SmallString to avoid a heap allocation.

therealprof commented 5 years ago

Or it could be queried once and then passed down to the users. ;)

gnzlbg commented 5 years ago

PRs welcome.

therealprof commented 5 years ago

Looking at the code it would probably be very useful to create a TargetInfo structure which is populated once and could be used in all the places in a defined manner, rather than "parsing" the target string everywhere but for now this makes it work much smoother for me.

therealprof commented 5 years ago

(smoother in the sense that I can hook into such target function rather than the small time savings 😉)