--timings: show crate version in the graph?

[matthiaskrgr]

When different dependencies pull in multiple versions of the same crate, it would help to see the version of the crate that was compiled. (See syn when building rustfmt

)

[m-bal]

I would like to take a crack at this. I understand the team doesn't accept new features without the S-accpeted label, but I thought I'd ask in case. Is it okay to work on this?

[weihanglo]

Hi, @m-bal. Thank you for being interested in this. I feel like we could display the version (or source) when they need to disambiguate. Some pointers:

• The units table is serialized here

  cargo/src/cargo/core/compiler/timings.rs

  Lines 564 to 568 in 27a41d6

  	unit.name_ver(),
  	unit.target,
  	unit.duration,
  	codegen,
  	features,

• The drawing of the SVG of an unit lives in

  cargo/src/cargo/core/compiler/timings.js

  Line 114 in 27a41d6

  const label = `${unit.name}${unit.target} ${unit.duration}s`;

• The unit data passed into JavaScript is here

  cargo/src/cargo/core/compiler/timings.rs

  Line 496 in 27a41d6

  name: ut.unit.pkg.name().to_string(),

[m-bal]

... when they need to disambiguate.

@weihanglo Do you mean only add the crate version if we see a duplicate crate (same crate name) with a different version in the list of crates (units)? If so, would creating a list of duplicate crates with a different version, then checking that list when creating the label and adding the version be a sufficient implementation?

[weihanglo]

Yep. unit.pkg or something else should be sufficient to disambiguate.

[remkop22]

I don't see any progress on this. Would it be okay if I picked this up?

[weihanglo]

@remkop22, not sure whether @m-bal already started or not. Since it was a month ago, it's probably fine for you to take over I guess.

Feel free to ask questions here!

[Angelin01]

@weihanglo Pardon my intrusion as I believe there's two people that said they'd fix this issue, but I took a quick look at the code you linked and I do believe the solution is as simple as this patch:

diff --git a/src/cargo/core/compiler/timings.js b/src/cargo/core/compiler/timings.js
index 986070ab0..92f57fb9a 100644
--- a/src/cargo/core/compiler/timings.js
+++ b/src/cargo/core/compiler/timings.js
@@ -111,7 +111,7 @@ function render_pipeline_graph() {
     ctx.textAlign = 'start';
     ctx.textBaseline = 'middle';
     ctx.font = '14px sans-serif';
-    const label = `${unit.name}${unit.target} ${unit.duration}s`;
+    const label = `${unit.name}${unit.target} (v${unit.version}) ${unit.duration}s`;
     const text_info = ctx.measureText(label);
     const label_x = Math.min(x + 5.0, canvas_width - text_info.width - X_LINE);
     ctx.fillText(label, label_x, y + BOX_HEIGHT / 2);

Which gives this result:

It doesn't only mark the version when there are duplicate crates, but in my opinion this is more "honest" as the version is shown at all times, making visual comparisons easier.
Is this acceptable?

[weihanglo]

Hey @Angelin01, thanks for being interested in this!

My original idea is that duplicate crates are minority, so it may be worth showing versions only when there are really duplicates to reduce visual noise. I don't really have a strong opinion, and it is easy to revert if people feel too noisy then.

So, go ahead!

(BTW I may display it as syn v2.0.18 since the table below uses this format. Having the same format make it easy to do text search)