Add support for combined stack + ustack DTrace collapse

[mkeeter]

I've been using flamegraph to get user + kernel flamegraphs, with a custom DTrace profiling command

flamegraph -c'profile-997 { @[stack(100)] = count(); } profile-997 { @[ustack(100)] = count(); }' ...

However, this puts the user + kernel stacks side by side, when in fact some of the user stacks are calling into the kernel:

It's possible to get a combined stack with

profile-997 { @[stack(100),ustack(100)] = count(); }

Sampling both stacks simultaneously means there's an extra newline between the kernel and user stacks, e.g.

              zfs`dmu_objset_ds+0x1
              zfs`zil_itx_assign+0x194
              zfs`zil_commit_itx_assign+0x65
              zfs`zil_commit_impl+0x26
              zfs`zil_commit+0x4b
              zfs`zfs_fsync+0xf6
              genunix`fop_fsync+0x4a
              genunix`fdsync+0xc4
              unix`sys_syscall+0x17d

              libc.so.1`__fdsync+0xa
              cru-ds-fdsync-v1`std::fs::File::sync_all::hcd4d0768a77cbc2e+0x14
              cru-ds-fdsync-v1`<crucible_downstairs::extent_inner_raw::RawInner as crucible_downstairs::extent::ExtentInner>::flush::h68fcf1774758d74e+0x9c
              cru-ds-fdsync-v1`crucible_downstairs::extent::Extent::flush::h0eac5b95dfa4f5f0+0x472
              cru-ds-fdsync-v1`std::panicking::try::h7ba6611983f64757+0x4c
              cru-ds-fdsync-v1`_$LT$rayon_core..job..HeapJob$LT$BODY$GT$$u20$as$u20$rayon_core..job..Job$GT$::execute::h60b9d586fc4f8a0f (.llvm.8052073739315931670)+0x46
              cru-ds-fdsync-v1`rayon_core::registry::WorkerThread::wait_until_cold::haa78671c0e7aa9b1+0x50f
              cru-ds-fdsync-v1`rayon_core::registry::ThreadBuilder::run::hf28d413d115bded0+0x398
              cru-ds-fdsync-v1`std::sys_common::backtrace::__rust_begin_short_backtrace::hfcd6324a3e87fc1e+0x48
              cru-ds-fdsync-v1`core::ops::function::FnOnce::call_once{{vtable.shim}}::hbfa4fc2e086997af+0xb2
              cru-ds-fdsync-v1`std::sys::unix::thread::Thread::new::thread_start::h1783cbcbbf061711+0x29
              libc.so.1`_thrp_setup+0x77
              libc.so.1`_lwp_start
                1

This PR updates the DTrace collapser to support stacks with this bonus newline. It's backwards compatible for ustack-only traces, and includes a new unit test for the new behavior.

[jonhoo]

Nice, thanks for contributing a fix!

[jonhoo]

This doesn't seem quite right. In particular, it means has_seen_digit won't be reset to false if we hit a return false earlier. Not sure if that's intentional?

[mkeeter]

Good catch, I realized that this is more complicated than necessary. f1e6fbf simplifies it and adds a unit test.

[jonhoo]

I wonder if we should do something to annotate that these come from calls in the kernel so that they can be given a different color in the flamegraph output given we have that information from DTrace? Feels like a shame to throw it away. What do you think?

[mkeeter]

There's certainly a bunch of data in here that we're not using, but I'd worry about overspecializing for a particular system.

My ideal system would be something like a config file that specifies regex → palette, or even a small DSL for generating a color.

[
  {
    "regex": "zfs`",
    "palette": "warm"
  }, 
  {
    "regex": "libc`",
    "palette": "cool"
  },
  {
    "regex": ".*::.*",
    "palette": "green"
  }
  ...etc
]

The language-specific color schemes could be represented with this format, but users could also bring their own config file for flexibility (without having to edit inferno itself)

Anyways, this is probably orthogonal to the PR 😄

[mkeeter]

@jonhoo I think this is good to go; the remaining CI failure is a Codeconv rate limit 😅

[jonhoo]

Thanks!

[jonhoo]

Releasing in #330