Deprecate vs Remove `stmt_html` output for Halide 16? #7507

steven-johnson · 2023-04-13T19:02:39Z

With #7421 having landed, there is really not much use for the stmt_html output any more.

Ordinarily I'd say we should deprecate it, but (1) it will be hard to emit useful deprecation warnings for all use cases, and (2) I don't recall seeing a single use of it inside Google for several years now -- I wouldn't be surprised if the same is true externally.

So, taking a straw poll here -- does it make more sense to just rip off the band-aid, so to speak, and remove it entirely for Halide 16?

The text was updated successfully, but these errors were encountered:

shoaibkamil · 2023-04-13T20:04:13Z

I think it's ok to remove it, but we may want to alias stmt_html to output stmt_viz or at least provide a reasonable error message-- but that seems just as hard as a deprecation message.

This takes the approach of "Remove completely" vs "Deprecate for Halide 16" -- it's not at all clear if this is what the consensus is, but putting it there for consideration.

slomp · 2023-04-14T02:20:15Z

One thing to consider: compared to stmt_html, how long does it take to generate? And also, how long does it take to open it in a browser?

abadams · 2023-04-14T21:50:03Z

I propose that to a user of the releases, it should just appear as if the html output got better. So we don't introduce any "viz" names at all (which already collide with HalideTraceViz), and just swap in the new html output and delete the old one at the same time.

steven-johnson · 2023-04-14T21:55:30Z

Tagging @maaz139 as his input would be welcomed here

maaz139 · 2023-04-14T22:44:09Z

One thing to consider: compared to stmt_html, how long does it take to generate? And also, how long does it take to open it in a browser?

The new HTML is definitely slower to load. In general, html files above ~15mb were not working well in my experiments. I think there are ways to improve the HTML performance and it would be worth doing so if people are actively using the feature and running into this issue.

Other than performance, I don't see a reason to keep the old html file around. I would advocate silently swapping the new viz.html for the old html tags as Andrew suggested.

antonysigma · 2023-04-17T17:08:51Z

Chiming in... As an Nvidia GPU programmer, I am in favor of the new StmtViz output as long as we can bring back the gpu_threads, gpu_blocks IR visualization back in Halide 10. Ever since I moved to Halide 14, all the scheduled GPU code are visualized as assembly code.

The strength of StmtViz tool is to compare the IR and assembly code side by side, with cross referencing. However, if the IR visualization panel dumps the assembly code like what we do in Halide 14, it kind of defeats the purpose, right?

The figure below shows how a generic code blurred_x , scheduled with gpu_theads, is visualized in Halide 10 and Halide 14.

maaz139 · 2023-04-17T17:29:12Z

Thanks for the feedback Antony! I agree, StmtViz should show IR code now that we have a separate assembly tab. I'll open an issue and look into it.

abadams · 2023-04-17T18:13:54Z

For that to happen we'd have to emit the .stmt file before the offload gpu pass, e.g. in Lower.cpp around line 441. I guess we'd have to stash that earlier stmt in the Module somewhere for use in the .stmt outputs.

Per discussion on #7507, this entirely removes the "classic" stmt_html output and replaces it with the "new" StmtToViz output. Using `compile_to_lowered_stmt` or requesting `stmt_html` will now always output the new output, and requesting `stmt_viz` output is no longer legal. (Note that this builds on top of #7515, which must be submitted first.) It's not clear to me whether #7507 (comment) is a blocker for this change, or a request to add back already-lost functionality.

* Allow emitting `stmt_viz` without specifying `assembly` TL;DR: if we request `stmt_viz` without `assembly`, just generate the latter to a temp file that we dispose of later; this wasn't feasible before since we were previously requiring the assembly output to be generated with the same directory and basename as stmt_viz, but that was fixed. * Convert stmt_html output to use stmt_viz output Per discussion on #7507, this entirely removes the "classic" stmt_html output and replaces it with the "new" StmtToViz output. Using `compile_to_lowered_stmt` or requesting `stmt_html` will now always output the new output, and requesting `stmt_viz` output is no longer legal. (Note that this builds on top of #7515, which must be submitted first.) It's not clear to me whether #7507 (comment) is a blocker for this change, or a request to add back already-lost functionality. * Update Makefile * Update Generator.cpp

steven-johnson · 2023-04-18T16:29:45Z

As written, this issue is fixed, closing. (I presume we'll open a new issue for @antonysigma's request)

mcourteaux · 2023-07-25T18:26:57Z

I want the original HTML statement file back. The new version is SUPER slow in the webbrowser.

maaz139 · 2023-07-25T18:41:07Z

Perhaps we can add lazy rendering for the visualization / assembly tabs. This should, in theory, restore the page responsiveness and load times to previous levels until the user explicitly requests the rendering of the new tabs.

mcourteaux · 2023-07-25T18:49:22Z

This message stays in the lower-left corner for me for a long while. When I check the network tab, it says that all the resources are cached, so I don't know what it's doing. Parsing the javascript file?

steven-johnson · 2023-07-25T19:11:25Z

Out of curiosity, what browser / hardware etc are you using? (I'm not aware that anyone else involved in the review of this noticed a meaningful slowdown, so it may be something specific to your setup that we can address somehow)

mcourteaux · 2023-07-25T19:28:43Z

Out of curiosity, what browser / hardware etc are you using? (I'm not aware that anyone else involved in the review of this noticed a meaningful slowdown, so it may be something specific to your setup that we can address somehow)

An 48GB memory, 8-core machine.

Running Mozilla Firefox 115.0.2

Lemme try this in Chromium real quick...
Same for Chromium: it took 12 seconds for the page to load.

Feel free to try this for yourself:
neonraw_denoiser_f8_k7_p1-x86-64-linux-avx2-fma-profile_by_timer.stmt.html.zip

antonysigma · 2023-07-25T20:27:12Z

Same for Chromium: it took 12 seconds for the page to load.

@mcourteaux I am afraid the Halide team (as well as me) are not too familiar with the Javascript and ES6. Have you considered reaching out to the original author of the PR?

Core HTML5 developers tend to bundle all scripts with toolchains like the esbuild or webpack, so as to embed the entire web application as one offline script file bundled.js.

Chiming in... Could you please post a screenshot of the waterfall diagram representing the load times? This is what I have on my machine, rendering the same HTML you uploaded earlier:

mcourteaux · 2023-07-26T08:57:50Z

In Chromium, I can record a profile:

(The waterfall for network resources is useless, it's instantly loaded. The bottleneck is indeed some script running).

The script in question is the one that starts inline at line 8898. The call stack is this very non-helpful thing...

mcourteaux · 2023-07-26T09:05:22Z

Aha! I caught a very useful profile!

All the time is spent in "init_tooltips" which is just waiting 100% of the time on jQuery selectors.

I'll upload it here. Trace-20230726T110312.json.zip
Extract, and open it with Chromium in the Performance tab of the Developer Tools.

mcourteaux · 2023-07-26T09:05:39Z

I'll move this to a new issue and tag the right people.

mcourteaux · 2023-07-26T09:13:48Z

Opened #7712

* Allow emitting `stmt_viz` without specifying `assembly` TL;DR: if we request `stmt_viz` without `assembly`, just generate the latter to a temp file that we dispose of later; this wasn't feasible before since we were previously requiring the assembly output to be generated with the same directory and basename as stmt_viz, but that was fixed. * Convert stmt_html output to use stmt_viz output Per discussion on halide#7507, this entirely removes the "classic" stmt_html output and replaces it with the "new" StmtToViz output. Using `compile_to_lowered_stmt` or requesting `stmt_html` will now always output the new output, and requesting `stmt_viz` output is no longer legal. (Note that this builds on top of halide#7515, which must be submitted first.) It's not clear to me whether halide#7507 (comment) is a blocker for this change, or a request to add back already-lost functionality. * Update Makefile * Update Generator.cpp

steven-johnson mentioned this issue Apr 14, 2023

Add a few missing support bits for StmtViz #7508

Closed

steven-johnson mentioned this issue Apr 17, 2023

Convert stmt_html output to use stmt_viz output #7516

Merged

steven-johnson closed this as completed Apr 18, 2023

abadams mentioned this issue Apr 18, 2023

stmt and stmt_html output are too low level #7519

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate vs Remove `stmt_html` output for Halide 16? #7507

Deprecate vs Remove `stmt_html` output for Halide 16? #7507

steven-johnson commented Apr 13, 2023

shoaibkamil commented Apr 13, 2023

slomp commented Apr 14, 2023

abadams commented Apr 14, 2023

steven-johnson commented Apr 14, 2023

maaz139 commented Apr 14, 2023 •

edited

Loading

antonysigma commented Apr 17, 2023 •

edited

Loading

maaz139 commented Apr 17, 2023

abadams commented Apr 17, 2023

steven-johnson commented Apr 18, 2023

mcourteaux commented Jul 25, 2023

maaz139 commented Jul 25, 2023

mcourteaux commented Jul 25, 2023

steven-johnson commented Jul 25, 2023

mcourteaux commented Jul 25, 2023

antonysigma commented Jul 25, 2023 •

edited

Loading

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

Deprecate vs Remove stmt_html output for Halide 16? #7507

Deprecate vs Remove stmt_html output for Halide 16? #7507

Comments

steven-johnson commented Apr 13, 2023

shoaibkamil commented Apr 13, 2023

slomp commented Apr 14, 2023

abadams commented Apr 14, 2023

steven-johnson commented Apr 14, 2023

maaz139 commented Apr 14, 2023 • edited Loading

antonysigma commented Apr 17, 2023 • edited Loading

maaz139 commented Apr 17, 2023

abadams commented Apr 17, 2023

steven-johnson commented Apr 18, 2023

mcourteaux commented Jul 25, 2023

maaz139 commented Jul 25, 2023

mcourteaux commented Jul 25, 2023

steven-johnson commented Jul 25, 2023

mcourteaux commented Jul 25, 2023

antonysigma commented Jul 25, 2023 • edited Loading

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

mcourteaux commented Jul 26, 2023

Deprecate vs Remove `stmt_html` output for Halide 16? #7507

Deprecate vs Remove `stmt_html` output for Halide 16? #7507

maaz139 commented Apr 14, 2023 •

edited

Loading

antonysigma commented Apr 17, 2023 •

edited

Loading

antonysigma commented Jul 25, 2023 •

edited

Loading