Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cargo timings graph is broken in Firefox #8850

Open
Tracked by #84
Lucretiel opened this issue Nov 11, 2020 · 7 comments
Open
Tracked by #84

Cargo timings graph is broken in Firefox #8850

Lucretiel opened this issue Nov 11, 2020 · 7 comments
Labels
A-timings Area: timings C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@Lucretiel
Copy link

When I run cargo +nightly -Z timings, it produces a timings report in HTML as expected. This renders correctly in Chrome and Safari. However, in Firefox, I only see the summary table and timings breakdown table. I can see the controls for the graph, but the graph itself doesn't render:
image

In the error console, I see the following uncaught exception:

Uncaught 
    Exception
        columnNumber: 0
        data: null
        filename: "http://localhost:8080/cargo-timing.html"
        lineNumber: 24173
        message: ""
        name: "NS_ERROR_FAILURE"
        result: 2147500037
        stack: "setup_canvas@http://localhost:8080/cargo-timing.html:24173:7\ndraw_graph_axes@http://localhost:8080/cargo-timing.html:24187:25\nrender_pipeline_graph@http://localhost:8080/cargo-timing.html:23930:86\n@http://localhost:8080/cargo-timing.html:24331:1\n"
        <prototype>: ExceptionPrototype { toString: toString(), name: Getter, message: Getter, … }

This exception is being thrown from the following function, specifically at ctx.scale(dpr, dpr) near the end:

function setup_canvas(id, width, height) {
  let g = document.getElementById(id);
  let dpr = window.devicePixelRatio || 1;
  g.width = width * dpr;
  g.height = height * dpr;
  g.style.width = width;
  g.style.height = height;
  let ctx = g.getContext('2d');
  ctx.scale(dpr, dpr);  // Exception thrown here
  return ctx;
}

Digging into the debugger, there don't seem to be any obvious issue with that line of code. ctx is of prototype CanvasRenderingContext2DPrototype, and the scale method has been supported in firefox pretty much forever. dpr == 2.

Meta

rustc --version --verbose:

cargo 1.49.0-nightly (79b397d72 2020-10-15)
release: 1.49.0
commit-hash: 79b397d72c557eb6444a2ba0dc00a211a226a35a
commit-date: 2020-10-15
  • macOS Catalina, version 10.15.7 (19H2)
  • Firefox 81.0.2 (64-bit)

@Lucretiel
Copy link
Author

Here's the complete HTML report triggering this issue:
cargo-timing.html.zip

@ehuss ehuss transferred this issue from rust-lang/rust Nov 11, 2020
@ehuss
Copy link
Contributor

ehuss commented Nov 11, 2020

Transferred to the cargo repo.

Set the scale slider to the far left and reload the page, and then slowly raise the scale slider up to whatever limit it can handle. The problem is that the canvas is too large. AFAIK, there isn't a way to get the maximum canvas size (I think it may be graphics-card dependent), so it's kinda hard to know how to handle that.

@ehuss ehuss added the A-timings Area: timings label Nov 11, 2020
@Lucretiel
Copy link
Author

Thanks, will give it a try

@Lucretiel
Copy link
Author

Yeah, that fixes it. Sliding the scale down doesn't do anything immediately, but refreshing afterwards does show the graphs.

@upsuper
Copy link
Contributor

upsuper commented Nov 10, 2022

I looked into this issue a bit. For me the rendering works until scale level 18, where canvas size being 4,876 x 25,406 = 123,879,656, and at scale level 19, where canvas size is 5,138 x 25,406 = 130,536,028, it stops rendering.

This is indeed a huge canvas, but not too crazy, and it's still smaller than what's documented on MDN that Firefox supports up to 472,907,776 px of area.

So I dug into Firefox source code a bit, and I believe this is not limited by something graphics card dependent, but a fixed value in the prefs, specifically gfx.max-alloc-size, which has a default value of 500,000,000 and its unit is bytes.

It can be seen that

  • 4,876 x 25,406 x 4 = 123,879,656 x 4 = 495,518,624 < 500,000,000
  • 5,138 x 25,406 x 4 = 130,536,028 x 4 = 522,144,112 > 500,000,000

to confirm that, I tried changing this value to 600,000,000 and restarted Firefox (as this value is read only once at startup), and indeed I could now get to scale level 21, where the canvas is 5,662 x 25,406, and failed again at level 22, where the canvas is 5,924 x 25,406. Repeat the computation above:

  • 5,662 x 25,406 x 4 = 143,848,772 x 4 = 575,395,088 < 600,000,000
  • 5,924 x 25,406 x 4 = 150,505,144 x 4 = 602,020,576 > 600,000,000

So it can be seen, this pref is the limiting factor.

Currently, the code that Cargo generates seems to try to limit the width of the graph in order to ensure that it can be rendered:

// Cap the size of the graph. It is hard to view if it is too large, and
// browsers may not render a large graph because it takes too much memory.
// 4096 is still ridiculously large, and probably won't render on mobile
// browsers, but should be ok for many desktop environments.
const graph_width = Math.min(scale * DURATION, 4096);

given that browsers have limitation on total area in addition to max single dimension, it might be better to limit it based on some area estimation rather than a hardcoded width limit.

Also, at this large canvas size, many browsers may have lost the graphics acceleration anyway, making the rendering very slow. I would suggest to have a canvas size relative to the viewport, and do windowing manually, rather than stressing browsers on their limits.

@weihanglo weihanglo added the C-bug Category: bug label Nov 10, 2022
@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed E-help-wanted labels Apr 18, 2023
@nagisa
Copy link
Member

nagisa commented Jan 17, 2024

For me the problem seems to have to do with the height of the canvas being more than 32k pixels. The graph won’t render until I filter out enough crates with the "min duration" slider. Here's the relevant bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1282074

I don’t think this can be fixed holistically by cosmetic changes to the canvas-based code. Rewriting the entire thing to DOM, SVG or if canvas must be kept, implementing some sort of scrolling and a rendering loop will be necessary.

@bbb651
Copy link

bbb651 commented Mar 7, 2024

I'm also in favor of a DOM/SVG based solution, it offers several immediate benefits:

  • Accessibility, the current graph is invisible to a screen reader
  • Making the text in the graph searchable
  • Allowing it to be easily themed with a prefers-color-scheme media query for a simple dark theme or with browser extensions
  • Not requiring javascript

There are existing tools that use both approaches, for example FlameGraph uses an SVG and speedscope uses an interactive canvas interface.
I think making use of an existing tool, by either utilizing an existing benchmark file format or embedding the viewer it in the generated html would provide the best experience, but either way would probably be as much work as rewriting the entire thing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-timings Area: timings C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

6 participants