Stream: t-compiler/wg-self-profile

Topic: breaking changes to summarize


simulacrum (Oct 02 2019 at 23:17, on Zulip):

@mw I believe something landed in the last few days that broke summarize; I'd appreciate a ping or so on the PRs to the measureme repo so I can update it on the perf server

simulacrum (Oct 02 2019 at 23:17, on Zulip):

I guess I can make it always update to latest

simulacrum (Oct 02 2019 at 23:17, on Zulip):

I'd just rather not

simulacrum (Oct 02 2019 at 23:18, on Zulip):

since that also seems error prone

simulacrum (Oct 02 2019 at 23:19, on Zulip):

hm actually it looks like summarize is just broken:

    stderr=    Checking issue-32278-big-array-of-strings v0.1.0 (/tmp/.tmpW1HBwV)
    thread 'main' panicked at 'assertion failed: `(left == right)`
      left: `"codegen_and_optimize_crate"`,
     right: `"codegen_crate"`', summarize/src/analysis.rs:121:17
    note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
    thread 'main' panicked at 'summarize failed in "/tmp/.tmpW1HBwV/self-profile-output"; prefix is "issue_32278_big_array_of_strings-12842"', collector/src/bin/rustc-fake.rs:96:21
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
    error: Could not compile `issue-32278-big-array-of-strings`.

    To learn more, run the command again with --verbose.
simulacrum (Oct 02 2019 at 23:21, on Zulip):

cc @Wesley Wiser

simulacrum (Oct 02 2019 at 23:21, on Zulip):

this is kind of painful

simulacrum (Oct 02 2019 at 23:22, on Zulip):

I guess we just won't get self profile data for some time

Wesley Wiser (Oct 03 2019 at 16:08, on Zulip):

Hmm... that's a little concerning. I don't recall anything landing recently that I would have broken this.

simulacrum (Oct 03 2019 at 16:11, on Zulip):

to be clear I'm seeing this on every single perf build

simulacrum (Oct 03 2019 at 16:11, on Zulip):

Have not tried to reproduce locally yet, ended up just disabling self profile on perf collection for now

Wesley Wiser (Oct 03 2019 at 16:16, on Zulip):

Most of the changes recently have been in the other tools so we should be able to rule them out

Wesley Wiser (Oct 03 2019 at 16:16, on Zulip):

That leaves

Wesley Wiser (Oct 03 2019 at 16:16, on Zulip):

https://github.com/rust-lang/measureme/commit/5b76a032daca7cd6e412a8dede6c535019067fff

Wesley Wiser (Oct 03 2019 at 16:17, on Zulip):

and
https://github.com/rust-lang/rust/pull/64840

simulacrum (Oct 03 2019 at 16:27, on Zulip):

I'm 99% sure its the rust-lang/rust PR because I only updated measureme on the perf server _after_ I noticed these errors, in the hope that they'd go away

Andreas Jonson (Oct 05 2019 at 18:12, on Zulip):

think that I have found what event causes the panic https://github.com/rust-lang/rust/pull/64840#discussion_r331756092

Andreas Jonson (Oct 05 2019 at 18:13, on Zulip):

but do not know how to solve it by any other way then remove the trace

mw (Oct 07 2019 at 08:04, on Zulip):

oh sorry, yes that rustc PR changed some of the event names. I didn't realize that summarize was relying a specific names there.

mw (Oct 07 2019 at 08:06, on Zulip):

Hm, OK now I see what the problem is.

mw (Oct 07 2019 at 08:07, on Zulip):

the backend actually is asynchronous there, so the weird nesting of events is correct ...

mw (Oct 07 2019 at 08:23, on Zulip):

I approved @Andreas Jonson's PR (https://github.com/rust-lang/rust/pull/65137) for in order to quickly unbreak summarize (thanks again!). I'll take a closer look as part of the second event review round.

mw (Oct 07 2019 at 08:23, on Zulip):

I wonder if we can prevent breakage like this in the future?

simulacrum (Oct 07 2019 at 11:49, on Zulip):

maybe we can run cargotest with self profile enabled and summarize that data? probably fairly low overhead time wise

Last update: Nov 17 2019 at 06:55UTC