All new impls of StableOrd and ToStableHashKey look good to me 👍

ToStableHashKey (especially when implementing it via HashStable) is a pretty heavy hammer and I've personally never been very happy about the existence of the ToStableHashKey to begin with. So my recommendation would be to not invest into it too much.

There's a few types that appeared to be missing StableOrd impls, most notably &str and the tuples (provided that the elements within the tuple are StableOrd themselves). I went ahead and just added the impl for these types - is that correct?

Yes, for the two instances you mention that is correct. I'll take a look at other impls shortly.

Also I'd just like to confirm - is it correct to use unstable sort when calling into_sorted_stable_ord (to_sorted_stable_ord but for UnordSet iterators) with items that are &str? If two &strs are Eq, it should be completely unnoticeable if unstable sort swaps them, correct? Just avoids an allocation ig.

Unstable sort is always correct in two cases:

• When we know that no two elements are equal, e.g. when the sequence to be sorted is the set of keys of a map or set.
• When the items have no observable difference, so that re-ordering them cannot make a difference anywhere. This is the case for &str (because we assume that we don't look at verbatim pointer values). Note though, that the entire item has to be considered here: if sorting a sequence of (&str, SomeOtherType) with the &str as the sort key, then unstable sorting would be allowed to introduce indeterminism.

I am not sure whether this is a rustc bug or a measureme bug so I'm reporting it here under the theory that it's more likely to be seen by the right people :)

I ran

x clean rustc_middle && MAGIC_EXTRA_RUSTFLAGS='-Zself-profile -Zself-profile-events=default,query-keys' x check --stage 0 rustc_middle

on https://github.com/rust-lang/rust/commit/ba6f5e3b4d60ea5a847cd4402cca594cd40b218f. That generated a .mm_profdata file that the measureme tools fail to parse:

; crox rustc_middle-2790234.mm_profdata
Could not parse `event_id`. Found ASCII control character in <text> at 112 in "erase_regions_ty^^impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>)
-> &'b mut DiagnosticBuilder<'a, ()>"

It looks like FnOnce impls are getting an extra newline generated at the end. That seems like a bug we should fix in rustc, but not such a bad bug that it should prevent measureme from generating a report - could we translated it to an ASCII space instead, maybe?

As it only seems that control chars is not allowed in the EventId texts is to allow for future tags I think excluding the whitespace control chars is ok. What I can see right now the only tag used is for arguments and is '\x1E'.

fixes #207

cc @michaelwoerister

OK, this looks good! Thanks again, @andjo403!

Nice! Thanks, @andjo403 🙂

I'll need to take a closer look at the entire file format, to make sure this doesn't have any unintended consequences. It's been a while since I worked with this code.

But I believe the suggested change matches the originally intended behavior.

I doubt, however, that newlines are affected by that.

What I mean is: I doubt that newlines really have a reserved meaning in the encoding -- so it's probably fine to relax the requirements here 🙂

I don't remember the details of the encoding but I do remember that it assigns special meaning to certain ASCII control characters in order to be space efficient. I doubt, however, that newlines are affected by that.

I can look into relaxing the checks here, but unless we consider it high priority, it will probably be a while before I get to that.

FWIW, I'm glad the hack has been removed 😉 I was always surprised that it apparently didn't cause trouble.

Hi! Sorry I didn't see this earlier.

The TL;DR of the current state is that there is no clear, unified approach for async crashdump debugging yet -- mostly (in my opinion) because async Rust is largely a set of protocols with hardly any implementation details prescribed.

Every piece of middleware has lots of freedom in how things are represented internally, while at the same time it's currently not possible to provide a common debugging interface (e.g. via traits) because debuggers don't understand Rust and can't run code in the debuggee (especially when dealing with crashdumps).

The work mentioned in the mid-year report was mostly about fixing general debuginfo bugs and issues in the Rust compiler. The compiler does generate debuginfo now that contains the information we need for some basic things (see e.g. this proof-of-concept implementation). However, we have not found a way of making a debuginfo-based approach maintainable. What we have so far would require separate implementations for each debugger and there is not good strategy for regression testing.

However, it looks like you are more interested in something that works with a running process. @jswrenn has done a lot of work in this area and wrote a comparison of different approaches at https://hackmd.io/@jswrenn/SkldX98Ci. I suspect that any high-performance support for traces would have to be built into a given executor framework.

I could imagine that LLVM is less able to merge debuginfo with the hack removed, which results in more duplicated debuginfo?

Let's see if that fixed the test for macOS. @bors r=cjgillot

The &str type is not available in rust-gdb.

When debugging using rust-gdb, local variables with &str type are pretty printed.

(gdb) p my_string_local
$6 = "my string"
(gdb) ptype my_string_local
type = struct &str {
  data_ptr: *mut u8,
  length: usize,
}

I expect that a raw pointer referring to the same memory location on the stack can be casted to &str and printed. However, gdb throws an exception:

(gdb) p *(($rsp + 168) as *mut &[u8])
$15 = &[*gdb*](len: 14) = {109, 121, 32, 115, 116, 114, 105, 110, 103}
(gdb) p *(($rsp + 168) as *mut &str)
No typed name 'str' in current context

https://github.com/rust-lang/rust/blob/8ee24f6ee04c1dc42ddcd99c57bb0a7a15df1309/src/etc/debugger_pretty_printers_common.py#L149-L151.

Workaround

You can use a helper function to cast and dereference the pointer:

unsafe fn print_str(ptr: *const &str) -> &str {
    return *ptr;
}

(gdb) p my_lib::print_str($rsp + 168)
$15 = "my string"

Meta

rustc --version --verbose
rustc 1.38.0 (625451e37 2019-09-23)
binary: rustc
commit-hash: 625451e376bb2e5283fc4741caa0a3e8a2ca4d54
commit-date: 2019-09-23
host: x86_64-unknown-linux-gnu
release: 1.38.0
LLVM version: 9.0

Thanks, @tromey!

So that emitting only dep-info does not depend on HIR lowering. See this test for example.

@rustbot ready

Performance looks fine, I'd say. The only regressions come from AST-heavy incr-unchanged tests where it makes a difference that we are now doing the change-detection work that the compiler invalidly skipped before.