Update measureme crate to 0.5.0

[michaelwoerister]

This PR updates the measureme self-profiling crate to the latest release. Heads up, this version changes the trace file format, so the summarize tool on perf.rlo needs to be updated to 0.5 too.

r? @Mark-Simulacrum
cc @wesleywiser

[michaelwoerister]

I'm trying to run perf.rlo locally with this and I'm getting errors like thread 'main' panicked at 'overflow when subtracting durations', src/libcore/option.rs:1190:5. I don't know yet where those originate, probably summarize, but it's probably prudent to hold off on merging this until we know more.

[Mark-Simulacrum]

I wanted to add support for choosing the summarize binary based on commit to perf, but haven't gotten around to it -- looking at it though, our queue is relatively empty (we've gone back in time to the 9th of November already). So once the bugs you've noted are fixed, I think we can just land this and I'll update summarize at the same time on perf.

Do you want me to investigate those bugs?

@bors try so we can test an official build as well without rebuilding the compiler

[bors]

⌛ Trying commit edcca15 with merge 25cf298cde6309198b899a137158b3f12adffb29...

[michaelwoerister]

Alright, it looks like the new implementation of summarize is a bit less robust than the previous one. Specifically, I get a crash here: https://github.com/rust-lang/measureme/blob/0.5.0/summarize/src/analysis.rs#L166

The reason is that data.self_time is zero because we are actually in a incr_comp_loading event. So instead of subtracting from data.self_time, we'd need to subtract from data.incremental_load_time. Doing this fixed the issue for me locally. I'm not sure if this is the cleanest solution. It's a manifestation of the problem described in rust-lang/measureme#75.

[michaelwoerister]

I posted a proposed fix at rust-lang/measureme#93. We don't need to do a bugfix release, I think, because summarize isn't on crates.io actually. Actually we should, it's just confusing otherwise.

[bors]

☀️ Try build successful - checks-azure
Build commit: 25cf298cde6309198b899a137158b3f12adffb29 (25cf298cde6309198b899a137158b3f12adffb29)

[michaelwoerister]

@Mark-Simulacrum: rust-lang/measureme#93 has landed, so summarize from the master branch should be able to handle the data generated by this PR.

I did not bump any versions for that fix. It's a bit weird that the 0.5.0 version of summarize cannot handle the data generated by the 0.5.0 version of measureme. On the other hand it is also a bit weird to bump measureme's version to 0.5.1 even though it is exactly the same code as 0.5.0. Should the entire set of crates in the measureme repo share a single version number in order to make it clear what is supposed to be compatible with what? Or maybe it should just be the major version number that is shared. Probably that...

Anyway, the bug in summarize is fixed, the PR should be good as it is.

[Mark-Simulacrum]

I personally expect we'll eventually want to pin to a specific commit of the measureme repository -- with all tools at that commit -- rather than have versions.

I'll try to make time to make the perf conversion along with the merge of this PR this week (likely tomorrow, but we'll see if I get to it today).

[Mark-Simulacrum]

@bors r+ rollup=never

Once this merges I'll try and get perf switched over quickly to the new code for summarize (I'm going with the manual approach for now).

Not allowing this to be rolled up just in case it's a performance regression itself (we can't readily test that on perf today).

[bors]

📌 Commit edcca15 has been approved by Mark-Simulacrum

[michaelwoerister]

I'll try to make time to make the perf conversion along with the merge of this PR this week (likely tomorrow, but we'll see if I get to it today).

Is there a chance to turn summarize (and possibly other tools) into a rustup component instead? That would make it trivial for perf.rlo (and for users) to keep compiler and tools in sync.

[Mark-Simulacrum]

I'm afraid I wouldn't know, but I agree that seems like the path forward. Certainly for now we can likely get away without doing so (since it's likely to be somewhat non-trivial). @cuviper has recently added a component, so maybe they can write up some instructions on forge.rust-lang.org or a mentoring issue or something like that?

[cuviper]

Maybe something like a rustc-dev-tools component? Or would this have wider use?

I'm not sure I know enough about components to really mentor someone though. You can grep rustc-dev to see what I did for that case, but that was mostly copying how rust-std is handled. Yours would look more like one of the tool components, I guess.

[Mark-Simulacrum]

I think perhaps, yes, though I'm not sure. It may make sense to try to get the ball rolling on that (I imagine we have quite a few tools that could be a good fit for such a component -- cargo-bisect-rustc, for one, jumps to mind).

I wasn't sure if you had done more than to just figure out how rust-std was handled.

I probably don't have the bandwidth myself to try to move the needle on a measureme/rustc-dev-tools component, but would be willing to do the review for a PR that tries to get the ball rolling.

[michaelwoerister]

This would potentially have wider use at some point but then we can just add it to the regular rustc component. For now the tools could be made part of the regular rustc-dev component? They wouldn't add much to the download size, I guess? summarize is ~900 KB when xz compressed. But a separate component for tools is fine too.

[bors]

⌛ Testing commit edcca15 with merge 59947fc...

[bors]

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 59947fc to master...