fix(stmt-html): Fix embedded Buffer processing performance issue. (#8748)

* fix(stmt-html): Fix embedded Buffer processing performance issue.

* Improve loading performance of the assembly file when generating Stmt HTML. Move-optimized replace_all: allow for no-copy execution when target string is not found.

* Avoid using inefficient std::string::append(iter, iter).

Author: mcourteaux

src/Bounds.cpp

@@ -184,11 +184,12 @@ class Bounds : public IRVisitor {
         Bounds *const self;
         BoundsLogger(Bounds *self, const char *pretty_function)
             : self(self) {
-            string name = replace_all(pretty_function, "(anonymous namespace)::", "");
-            name = replace_all(name, "virtual void Halide::Internal::", "");
-            name = replace_all(name, "(const Halide::Internal::", "(");
-            name = replace_all(name, "::visit", "");
-            name = replace_all(name, " *)", ")");
+            string name = pretty_function;
+            name = replace_all(std::move(name), "(anonymous namespace)::", "");
+            name = replace_all(std::move(name), "virtual void Halide::Internal::", "");
+            name = replace_all(std::move(name), "(const Halide::Internal::", "(");
+            name = replace_all(std::move(name), "::visit", "");
+            name = replace_all(std::move(name), " *)", ")");
             log_line(name, " {");
             self->log_indent++;
         }
@@ -2220,16 +2221,17 @@ class BoxesTouched : public IRGraphVisitor {
 
         BoxesTouchedLogger(BoxesTouched *self, const char *pretty_function)
             : self(self), parent_logger(self->current_logger), boxes(self->boxes) {
-            string name = replace_all(pretty_function, "(anonymous namespace)::", "");
-            name = replace_all(name, "virtual void Halide::Internal::", "");
-            name = replace_all(name, "(const Halide::Internal::", "(");
-            name = replace_all(name, "::visit", "");
-            name = replace_all(name, " *)", ")");
+            string name = pretty_function;
+            name = replace_all(std::move(name), "(anonymous namespace)::", "");
+            name = replace_all(std::move(name), "virtual void Halide::Internal::", "");
+            name = replace_all(std::move(name), "(const Halide::Internal::", "(");
+            name = replace_all(std::move(name), "::visit", "");
+            name = replace_all(std::move(name), " *)", ")");
 
             if (self->consider_calls && !self->consider_provides) {
-                name = replace_all(name, "BoxesTouched", "BoxesRequired");
+                name = replace_all(std::move(name), "BoxesTouched", "BoxesRequired");
             } else if (!self->consider_calls && self->consider_provides) {
-                name = replace_all(name, "BoxesTouched", "BoxesProvided");
+                name = replace_all(std::move(name), "BoxesTouched", "BoxesProvided");
             }
 
             log_line(name, " {");

src/CompilerLogger.cpp

@@ -179,8 +179,8 @@ std::ostream &emit_value(std::ostream &o, const VALUE &value) {
 template<>
 std::ostream &emit_value<std::string>(std::ostream &o, const std::string &value) {
     std::string v = value;
-    v = replace_all(v, "\\", "\\\\");
-    v = replace_all(v, "\"", "\\\"");
+    v = replace_all(std::move(v), "\\", "\\\\");
+    v = replace_all(std::move(v), "\"", "\\\"");
     o << "\"" << v << "\"";
     return o;
 }

src/Module.cpp

@@ -223,10 +223,10 @@ static Registerer registerer;
             }
             std::string nsreg = "halide_nsreg_" + replace_all(f.name, "::", "_");
             std::string s = replace_all(registration_template, "$NAMESPACEOPEN$", nsopen);
-            s = replace_all(s, "$SHORTNAME$", simple_name);
-            s = replace_all(s, "$NAMESPACECLOSE$", nsclose);
-            s = replace_all(s, "$FULLNAME$", f.name);
-            s = replace_all(s, "$NREGS$", nsreg);
+            s = replace_all(std::move(s), "$SHORTNAME$", simple_name);
+            s = replace_all(std::move(s), "$NAMESPACECLOSE$", nsclose);
+            s = replace_all(std::move(s), "$FULLNAME$", f.name);
+            s = replace_all(std::move(s), "$NREGS$", nsreg);
             stream << s;
         }
     }
@@ -305,15 +305,15 @@ inline void apply_schedule_$SHORTNAME$(
         target_string += t.to_string();
     }
     std::string body_text = indent_string(body, "    ");
-    s = replace_all(s, "$SCHEDULER$", scheduler_name);
-    s = replace_all(s, "$NAMESPACEOPEN$", nsopen);
-    s = replace_all(s, "$SHORTNAME$", simple_name);
-    s = replace_all(s, "$CLEANNAME$", clean_name);
-    s = replace_all(s, "$NAMESPACECLOSE$", nsclose);
-    s = replace_all(s, "$TARGET$", target_string);
-    s = replace_all(s, "$BODY$", body_text);
-    s = replace_all(s, "$MPNAME$", "autoscheduler_params");
-    s = replace_all(s, "$MACHINEPARAMS$", autoscheduler_params_string);
+    s = replace_all(std::move(s), "$SCHEDULER$", scheduler_name);
+    s = replace_all(std::move(s), "$NAMESPACEOPEN$", nsopen);
+    s = replace_all(std::move(s), "$SHORTNAME$", simple_name);
+    s = replace_all(std::move(s), "$CLEANNAME$", clean_name);
+    s = replace_all(std::move(s), "$NAMESPACECLOSE$", nsclose);
+    s = replace_all(std::move(s), "$TARGET$", target_string);
+    s = replace_all(std::move(s), "$BODY$", body_text);
+    s = replace_all(std::move(s), "$MPNAME$", "autoscheduler_params");
+    s = replace_all(std::move(s), "$MACHINEPARAMS$", autoscheduler_params_string);
     stream << s;
 }
 
@@ -692,12 +692,7 @@ void Module::compile(const std::map<OutputFileType, std::string> &output_files)
         debug(1) << "Module.compile(): device_code " << output_files.at(OutputFileType::device_code) << "\n";
         Buffer<> buf = get_device_code_buffer();
         if (buf.defined()) {
-            int length = buf.size_in_bytes();
-            while (length > 0 && ((const char *)buf.data())[length - 1] == '\0') {
-                length--;
-            }
-            std::string str((const char *)buf.data(), length);
-            std::string device_code = std::string((const char *)buf.data(), buf.size_in_bytes());
+            std::string_view device_code((const char *)buf.data(), buf.size_in_bytes());
             while (!device_code.empty() && device_code.back() == '\0') {
                 device_code = device_code.substr(0, device_code.length() - 1);
             }

src/StmtToHTML.cpp

@@ -555,28 +555,40 @@ class AssemblyInfo : public IRVisitor {
         m.get_conceptual_stmt().accept(this);
     }
 
-    void generate(const std::string &code) {
+    void generate(std::string_view code) {
         // Find markers in asm code
-        std::istringstream asm_stream(code);
-        std::string line;
         int lno = 1;
-        while (getline(asm_stream, line)) {
-            // Try all markers
-            std::vector<uint64_t> matched_nodes;
-            for (auto const &[node, marker] : markers) {
-                if (std::regex_search(line, marker)) {
-                    // Save line number
-                    lnos[node] = lno;
-                    // Save this node's id
-                    matched_nodes.push_back(node);
-                }
+        size_t start = 0;
+
+        std::vector<uint64_t> matched_nodes;
+        while (start < code.size()) {
+            size_t end = code.find('\n', start);
+            if (end == std::string_view::npos) {
+                end = code.size();
             }
-            // We map to the first match, stop
-            // checking matched nodes
-            for (auto const &node : matched_nodes) {
-                markers.erase(node);
+            std::string_view line = code.substr(start, end - start);
+            std::string_view marker_prefix("%\"", 2);
+
+            // Quick check if the line contains %".
+            if (line.find(marker_prefix) != std::string_view::npos) {
+                // Try all markers
+                matched_nodes.clear();
+                for (auto const &[node, marker] : markers) {
+                    if (std::regex_search(line.begin(), line.end(), marker)) {
+                        // Save line number
+                        lnos[node] = lno;
+                        // Save this node's id
+                        matched_nodes.push_back(node);
+                    }
+                }
+                // We map to the first match, stop
+                // checking matched nodes
+                for (auto const &node : matched_nodes) {
+                    markers.erase(node);
+                }
             }
 
+            start = end + 1;
             lno++;
         }
     }
@@ -785,12 +797,12 @@ class HTMLCodePrinter : public IRVisitor {
     }
 
     std::string escape_html(std::string src) {
-        src = replace_all(src, "&", "&amp;");
-        src = replace_all(src, "<", "&lt;");
-        src = replace_all(src, ">", "&gt;");
-        src = replace_all(src, "\"", "&quot;");
-        src = replace_all(src, "/", "&#x2F;");
-        src = replace_all(src, "'", "&#39;");
+        src = replace_all(std::move(src), "&", "&amp;");
+        src = replace_all(std::move(src), "<", "&lt;");
+        src = replace_all(std::move(src), ">", "&gt;");
+        src = replace_all(std::move(src), "\"", "&quot;");
+        src = replace_all(std::move(src), "/", "&#x2F;");
+        src = replace_all(std::move(src), "'", "&#39;");
         return src;
     }
 
@@ -850,29 +862,29 @@ class HTMLCodePrinter : public IRVisitor {
                 scope.pop(current_kernel);
             }
 
-            line = replace_all(line, ".f32", ".<span class='OpF32'>f32</span>");
-            line = replace_all(line, ".f64", ".<span class='OpF64'>f64</span>");
+            line = replace_all(std::move(line), ".f32", ".<span class='OpF32'>f32</span>");
+            line = replace_all(std::move(line), ".f64", ".<span class='OpF64'>f64</span>");
 
-            line = replace_all(line, ".s8", ".<span class='OpI8'>s8</span>");
-            line = replace_all(line, ".s16", ".<span class='OpI16'>s16</span>");
-            line = replace_all(line, ".s32", ".<span class='OpI32'>s32</span>");
-            line = replace_all(line, ".s64", ".<span class='OpI64'>s64</span>");
+            line = replace_all(std::move(line), ".s8", ".<span class='OpI8'>s8</span>");
+            line = replace_all(std::move(line), ".s16", ".<span class='OpI16'>s16</span>");
+            line = replace_all(std::move(line), ".s32", ".<span class='OpI32'>s32</span>");
+            line = replace_all(std::move(line), ".s64", ".<span class='OpI64'>s64</span>");
 
-            line = replace_all(line, ".u8", ".<span class='OpI8'>u8</span>");
-            line = replace_all(line, ".u16", ".<span class='OpI16'>u16</span>");
-            line = replace_all(line, ".u32", ".<span class='OpI32'>u32</span>");
-            line = replace_all(line, ".u64", ".<span class='OpI64'>u64</span>");
+            line = replace_all(std::move(line), ".u8", ".<span class='OpI8'>u8</span>");
+            line = replace_all(std::move(line), ".u16", ".<span class='OpI16'>u16</span>");
+            line = replace_all(std::move(line), ".u32", ".<span class='OpI32'>u32</span>");
+            line = replace_all(std::move(line), ".u64", ".<span class='OpI64'>u64</span>");
 
-            line = replace_all(line, ".b8", ".<span class='OpB8'>b8</span>");
-            line = replace_all(line, ".b16", ".<span class='OpB16'>b16</span>");
-            line = replace_all(line, ".b32", ".<span class='OpB32'>b32</span>");
-            line = replace_all(line, ".b64", ".<span class='OpB64'>b64</span>");
+            line = replace_all(std::move(line), ".b8", ".<span class='OpB8'>b8</span>");
+            line = replace_all(std::move(line), ".b16", ".<span class='OpB16'>b16</span>");
+            line = replace_all(std::move(line), ".b32", ".<span class='OpB32'>b32</span>");
+            line = replace_all(std::move(line), ".b64", ".<span class='OpB64'>b64</span>");
 
-            line = replace_all(line, ".v2", ".<span class='OpVec2'>v2</span>");
-            line = replace_all(line, ".v4", ".<span class='OpVec4'>v4</span>");
+            line = replace_all(std::move(line), ".v2", ".<span class='OpVec2'>v2</span>");
+            line = replace_all(std::move(line), ".v4", ".<span class='OpVec4'>v4</span>");
 
-            line = replace_all(line, "ld.", "<span class='Memory'>ld</span>.");
-            line = replace_all(line, "st.", "<span class='Memory'>st</span>.");
+            line = replace_all(std::move(line), "ld.", "<span class='Memory'>ld</span>.");
+            line = replace_all(std::move(line), "st.", "<span class='Memory'>st</span>.");
 
             size_t idx;
             if ((idx = line.find("&#x2F;&#x2F")) != std::string::npos) {
@@ -2306,7 +2318,7 @@ class PipelineHTMLInspector {
         // use comments in the generated assembly to infer association
         // between Halide IR and assembly -- unclear how reliable this is.
         host_asm_info.gather_nodes_from_functions(m);
-        host_asm_info.generate(asm_stream.str());
+        host_asm_info.generate(asm_buffer);
 
         Buffer<> device_code_buf = m.get_device_code_buffer();
         if (device_code_buf.defined()) {
@@ -2315,7 +2327,7 @@ class PipelineHTMLInspector {
             debug(1) << "Generating device AssemblyInfo\n";
             // TODO(mcourteaux): This doesn't generate anything useful, as the
             // LLVM comments are only added later in the LLVM CodeGen IRVisitor.
-            // This conceptual Stmt hasn't seen this seen this
+            // This conceptual Stmt hasn't seen this pass yet.
             device_asm_info.gather_nodes_from_conceptual_stmt(m);
             device_asm_info.generate(device_assembly);
         } else {
@@ -2460,15 +2472,18 @@ class PipelineHTMLInspector {
         stream << "<div id='host-assembly-pane' class='pane'>\n";
         stream << "<div id='assemblyContent' class='shj-lang-asm'>\n";
         stream << "<pre>\n";
-        std::istringstream ss{asm_stream.str()};
-        for (std::string line; std::getline(ss, line);) {
-            if (line.length() > 500) {
-                // Very long lines in the assembly are typically the _gpu_kernel_sources
-                // as a raw ASCII block in the assembly. Let's chop that off to make
-                // browsers faster when dealing with this.
-                line = line.substr(0, 100) + "\" # omitted the remainder of the ASCII buffer";
+        // The loop below is preferred to avoid copying the data
+        // again into a new std::istringstream.
+        std::string_view asm_str = asm_buffer;
+        size_t start = 0;
+        while (start < asm_str.size()) {
+            size_t end = asm_str.find('\n', start);
+            if (end == std::string_view::npos) {
+                end = asm_str.size();
             }
-            stream << html_code_printer.escape_html(line) << "\n";
+            std::string line{asm_str.substr(start, end - start)};
+            stream << html_code_printer.escape_html(std::move(line)) << "\n";
+            start = end + 1;
         }
         stream << "\n";
         stream << "</pre>\n";
@@ -2517,7 +2532,7 @@ class PipelineHTMLInspector {
     /* Misc helper methods */
 
     // Load assembly code from file
-    std::ostringstream asm_stream;
+    std::string asm_buffer;
     AssemblyInfo host_asm_info;
     AssemblyInfo device_asm_info;
 
@@ -2528,10 +2543,26 @@ class PipelineHTMLInspector {
         std::ifstream assembly;
         assembly.open(asm_file.c_str());
 
-        // Slurp the code into asm_stream
+        // Try to get size of the file for fewer allocations
+        asm_buffer.clear();
+        size_t file_size = assembly.seekg(0, std::ios::end).tellg();
+        asm_buffer.reserve(file_size + 16);
+        assembly.seekg(0);
+
+        // Slurp the code into asm_stream...
         std::string line;
+        line.reserve(128);
         while (getline(assembly, line)) {
-            asm_stream << line << "\n";
+            if (line.length() > 500) {
+                // Very long lines in the assembly are typically the _gpu_kernel_sources
+                // or other buffers (such as static LUTs) as a raw ASCII block in the
+                // assembly. Let's chop that off to make browsers faster when dealing with this.
+                asm_buffer.append(line.data(), 200);
+                asm_buffer.append("\" # omitted the remainder of the buffer\n");
+            } else {
+                asm_buffer.append(line);
+                asm_buffer.push_back('\n');
+            }
         }
     }
 };

src/Target.cpp

@@ -1133,7 +1133,7 @@ std::string Target::to_string() const {
     // Use has_feature() multiple times (rather than features_any_of())
     // to avoid constructing a temporary vector for this rather-common call.
     if (has_feature(Target::TraceLoads) && has_feature(Target::TraceStores) && has_feature(Target::TraceRealizations)) {
-        result = Internal::replace_all(result, "trace_loads-trace_realizations-trace_stores", "trace_all");
+        result = Internal::replace_all(std::move(result), "trace_loads-trace_realizations-trace_stores", "trace_all");
     }
     if (vector_bits != 0) {
         result += "-vector_bits_" + std::to_string(vector_bits);

src/Util.cpp

@@ -262,14 +262,13 @@ bool ends_with(const string &str, const string &suffix) {
     return true;
 }
 
-string replace_all(const string &str, const string &find, const string &replace) {
+string replace_all(string str, const string &find, const string &replace) {
     size_t pos = 0;
-    string result = str;
-    while ((pos = result.find(find, pos)) != string::npos) {
-        result.replace(pos, find.length(), replace);
+    while ((pos = str.find(find, pos)) != string::npos) {
+        str.replace(pos, find.length(), replace);
         pos += replace.length();
     }
-    return result;
+    return str;
 }
 
 std::vector<std::string> split_string(const std::string &source, const std::string &delim) {
@@ -388,7 +387,7 @@ std::string get_windows_tmp_dir() {
     std::string tmp = from_utf16(wlocal_path);
     CoTaskMemFree(wlocal_path);
 
-    tmp = replace_all(tmp, "\\", "/");
+    tmp = replace_all(std::move(tmp), "\\", "/");
     if (tmp.back() != '/') tmp += '/';
     tmp += "Temp/";
     return tmp;

src/Util.h

@@ -176,8 +176,12 @@ bool starts_with(const std::string &str, const std::string &prefix);
 /** Test if the first string ends with the second string */
 bool ends_with(const std::string &str, const std::string &suffix);
 
-/** Replace all matches of the second string in the first string with the last string */
-std::string replace_all(const std::string &str, const std::string &find, const std::string &replace);
+/** Replace all matches of the second string in the first string with the last string.
+ * The string to search-and-replace in is passed by value, offering the ability to
+ * std::move() a string in if you're not interested in keeping the original string.
+ * This is useful when the original string does not contain the find-string, causing
+ * this function to return the same string without any copies being made. */
+std::string replace_all(std::string str, const std::string &find, const std::string &replace);
 
 /** Split the source string using 'delim' as the divider. */
 std::vector<std::string> split_string(const std::string &source, const std::string &delim);

test/correctness/pytorch.cpp

@@ -6,16 +6,6 @@ using namespace Halide;
 
 namespace {
 
-std::string replace_all(const std::string &str, const std::string &find, const std::string &replace) {
-    size_t pos = 0;
-    std::string result = str;
-    while ((pos = result.find(find, pos)) != std::string::npos) {
-        result.replace(pos, find.length(), replace);
-        pos += replace.length();
-    }
-    return result;
-}
-
 std::string read_entire_file(const std::string &pathname) {
     std::ifstream f(pathname, std::ios::in | std::ios::binary);
     std::string result;
@@ -31,7 +21,7 @@ std::string read_entire_file(const std::string &pathname) {
     }
     f.close();
     // Normalize file to Unix line endings
-    result = replace_all(result, "\r\n", "\n");
+    result = Internal::replace_all(result, "\r\n", "\n");
     return result;
 }