Skip to content

Commit

Permalink
Auto merge of #12420 - Angelin01:timings-show-version, r=weihanglo
Browse files Browse the repository at this point in the history
Display crate version on timings graph

### What does this PR try to resolve?

Fixes #7384

The solution here is the simplest one: since duplicate crates with different versions can cause confusion, we always output the version.

### How should we test and review this PR?

Build any create using the `--timings` option:
```
cargo build --timings
```
and verify the resulting HTML file.

### Additional information

The formatting is consistent with how the crates are displayed in the terminal output for Cargo and also on the table below the graph. Initially, [this comment](#7384 (comment)) suggested said formatting for ease of searching, but I do believe the browser can't search in the graph itself.

Sample console output:
```
   Compiling lazycell v1.3.0
   Compiling unicode-xid v0.2.4
   Compiling unicode-width v0.1.10
   Compiling glob v0.3.1
   Compiling curl-sys v0.4.65+curl-8.2.1
   Compiling curl v0.4.44
   Compiling git2 v0.17.2
```

Sample rendered HTML output:
![image](https://github.com/rust-lang/cargo/assets/17818024/23aae84e-5107-4352-8d0f-ecdfcac59187)

### Possible issues and future work

A possible issue in this solution is that the output becomes too noisy. Other possible solutions:
- Only display the version if there are two "units" with the same name. One possible implementation is to count the names, aggregate them in a simple map and look it up during rendering.
- Create a small selection to select the disambiguate method between "Never", "Always" and "Only duplicates". This may be overkill.
  • Loading branch information
bors committed Aug 1, 2023
2 parents 9a7c1fe + da0fab3 commit 2da47eb
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/cargo/core/compiler/timings.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ function render_pipeline_graph() {
ctx.translate(X_LINE, MARGIN);

// Compute x,y coordinate of each block.
// We also populate a map with the count of each unit name to disambiguate if necessary
const unitCount = new Map();
UNIT_COORDS = {};
for (i=0; i<units.length; i++) {
let unit = units[i];
Expand All @@ -86,6 +88,10 @@ function render_pipeline_graph() {
}
let width = Math.max(px_per_sec * unit.duration, 1.0);
UNIT_COORDS[unit.i] = {x, y, width, rmeta_x};

const tag = `${unit.name}${unit.target}`;
const count = unitCount.get(tag) || 0;
unitCount.set(tag, count + 1);
}

// Draw the blocks.
Expand All @@ -111,7 +117,11 @@ 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 tag = `${unit.name}${unit.target}`;
const labelName = (unitCount.get(tag) || 0) > 1 ? `${unit.name} v${unit.version}${unit.target}` : tag;
const label = `${labelName} ${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);
Expand Down

0 comments on commit 2da47eb

Please sign in to comment.