Speedup the VizIR HTML.

[mcourteaux]

Read comments below to follow along. Overall, for my large Stmt HTML file, I got the time down from close to 15s to less than 2s.

[mcourteaux]

First commit does this:

From:

To:

[mcourteaux]

This CodeMirror thing to just show (not edit!) the assembly (without syntax highlighting) is super expensive and slow. Added functionality is low, IMHO.
It tries to be smart about inserting and removing text as you scroll, which causes the layout engine to recalculate the entire page layout, instead of simply scrolling through it. It introduces frame stutters of 150ms on my machine.

[mcourteaux]

Second commit results in this:

Notice how the giant (green) call-stack of the depth() calls is completely collapsed into a thin line.

[mcourteaux]

Shuffles were put by default in a "Block" div, which is weird and wrong, as those are expressions, not statements. Example of such a weird representation due to a "Block" div:

After my fix:

Additionally, LetStmt's used <p class="WrapLine"> to define the line wrapping behaviour, but it puts the contents of it inside it, which contains <div>s, which is not allowed:
https://stackoverflow.com/questions/8397852/why-cant-the-p-tag-contain-a-div-tag-inside-it
Therefore, I removed the use of <p> and consolidated it with the <div> for line wrapping.

Further, I removed the use of CodeMirror, and use a lightweight, very fast syntax highlighter called Speed-Highlight, which actually also colors the code nicely:

Jump to assembly still works.

I did some code reordering to reduce additional layout recomputations as well.
During my debugging process, I added a print_ln() to break up the stream of html, as it was unreadable, and not one of my text editors managed to handle this million-character-long line well.

Additionally, I flattened out the LetStmt DOM. Normally, the "body" of the LetStmt goes in as a child of the LetStmt div, which reflects the structure of the IR tree. However, this creates extremely deep DOMs, so I moved that out.

As a bonus, I cleaned up the dragging of the "resizerBars" that separates the 3 panes. Every time you clicked the bar, a NEW event listener is added to the document. This is now fixed.

As a second bonus, instead of making the "Jump To IR Viz" button jump to the label, I jumped to the parent box, which is much clearer once you move it.

Overall performance profile now looks like this upon loading:

[mcourteaux]

By the way, question: there seems to be two instances of bootstrap loaded:

Halide/src/irvisualizer/StmtToViz_dependencies.template.html

Lines 4 to 7 in c9bf3b1

	<script src='https://cdn.jsdelivr.net/npm/bootstrap@5.2.0/dist/js/bootstrap.bundle.min.js' integrity='sha384-A3rJD856KowSb7dwlZdYEkO39Gagi7vIsF0jrRAoQmDKKtQBHUuLZ9AsSv4jD4Xa' crossorigin='anonymous'></script>
	<link rel='stylesheet' href='https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/css/bootstrap.min.css'>
	<link rel='stylesheet' href='https://cdn.jsdelivr.net/npm/bootstrap-icons@1.5.0/font/bootstrap-icons.css'>

Once bootstrap.bundle.min.js and once bootstrap.min.js. Is this normal?

[antonysigma]

Tagging @darya-ver , the original author of #7056 .

[mcourteaux]

Once bootstrap.bundle.min.js and once bootstrap.min.js. Is this normal?

https://stackoverflow.com/a/60665404/155137

They seem to contain the same base, but the bundle has extras. When I have some time, I might try it without the bundle, to see if the more ligthweight version works.

Additionally, I would like to check if I can add the functionality to immediately jump to the assembly as well, straight from the statement code (instead of having to go through the IR viz pane).

[steven-johnson]

(I assume this is still a work-in-progress?)

[mcourteaux]

Not actively working on it anymore. I propose we merge this, and if I get to it again, and I add that feature (if possible) to jump directly from IR to assembly, I'll open another PR.

[steven-johnson]

LGTM pending green

[antonysigma]

Hi @mcourteaux ,

Chiming in... What prevents us from nesting the tooltip tag inside the cost-btn tag, so that we can avoid implementing init_tooltips() in Javascript?

The nested approach seems to enable a pure CSS implementation, completely eliminating the overhead you indicated earlier. Example.

--- StmtToViz.cpp	2023-08-03 09:44:27.598731408 -0700
+++ StmtToViz-1.cpp	2023-08-03 09:53:59.492905354 -0700
@@ -1260,14 +1260,13 @@
         }
 
         stream << "<div id='" << id << "' class='cost-btn CostColor" << line_costc << "'"
-               << "   aria-describedby='tooltip-" << id << "'"
                << "   line-cost='" << line_cost << "' block-cost='" << block_cost << "'"
                << "   line-cost-color='" << line_costc << "' block-cost-color='" << block_costc << "'>"
-               << "</div>";
-
-        stream << "<span id='tooltip-" << id << "' class='tooltip cond-tooltop' role='tooltip-" << id << "'>"
+               << "<span id='tooltip-" << id << "' class='tooltip cond-tooltop' role='tooltip-" << id << "'>"
                << prefix << line_cost
                << "</span>";
+               << "</div>";
+
     }
 
     /* Misc utility methods */

[mcourteaux]

What prevents us from nesting the tooltip tag inside the cost-btn tag, so that we can avoid implementing init_tooltips() in Javascript?

I definitely like this, however, the current implementation supports clicking some things, such that the tooltip stays, instead of only being visible on hover. I personally don't see the use in that, so I would be definitely in favor of a pure-CSS approach.

[mcourteaux]

Also, I believe the tooltips are used to accumulate the cost of the whole subtree under the node that it currently serves as a tooltip for. This tooltip value changes depending on whether the node is collapsed or expanded. Expanded only shows the "self" value, and when collapsed it shows the full subtree cost. This is not CSS-onlyable.

[mcourteaux]

Although, that doesn't prevent the tooltip appearing/disappearing mechanism to be fully CSS. It's just the content of the tooltip that is determined by the JS code. Not a bad idea after all, I think.

[mcourteaux]

@steven-johnson I don't see what the build issue is on the buildbot/Windows. Am I the blame? I really can't tell from that output.

[steven-johnson]

No, that Windows buildbot is just flaky bag of poo, but we don't currently have any budget to replace it. (Donations welcome!) Ready to land, thanks!

[mcourteaux]

I would love to hear if anybody feels the speed difference. Open an HTML stmt-file you might already have, then regenerate using a new Halide build with the merged PR, rebuild your generator, and rerun the generator and reopen the new file. For me, it literally went from 15+ seconds to 1 to 2 seconds for heavy files.

[maaz139]

Wonderful -- I have some benchmarks that I tested the original implementation on. I'll review this PR and post benchmark results over the weekend!

[maaz139]

@mcourteaux The rendering is definitely much faster now. Prior to this PR, chrome failed to load the html file for the resnet_50 app. At least for my machine, most html files larger than 15-20mb would struggle to render on Chrome. Resnet_50 now loads in about 5 seconds and is quite responsive!

[mcourteaux]

Amazing news! 🥳 It still is for like a second unresponsive every time you resize the panes, but I couldn't figure out how to fix that.

[abadams]

Tagging this with release notes so that we remember to list the improvements to the visualizer.