jinja : fix quadratic cost in gather_string_parts (#27034)

* jinja : fix quadratic cost in gather_string_parts

* fix some comments

* remove test

diff --git a/common/jinja/runtime.cpp b/common/jinja/runtime.cpp
index 474129df2c4c3f669c2b976b2b30e47a42124891..4ce79e32aa7cd845826cdeb1c3ef8e8418857e27 100644 (file)
--- a/common/jinja/runtime.cpp
+++ b/common/jinja/runtime.cpp
@@ -263,7 +263,7 @@ value binary_expression::execute_impl(context & ctx) {
             return res;
         }
         for (int64_t i = 0; i < repeat; ++i) {
-            res->val_str = res->val_str.append(str);
+            res->val_str.append(str);
         }
         return res;
     }

diff --git a/common/jinja/runtime.h b/common/jinja/runtime.h
index 0884a15922bb9bf6fcf254a881550ebcd6034174..69bd683c68f8628831c1b2d44c8c0fb5a16c089b 100644 (file)
--- a/common/jinja/runtime.h
+++ b/common/jinja/runtime.h
@@ -763,14 +763,22 @@ struct runtime {
         gather_string_parts_recursive(val, parts);
         // join consecutive parts with the same type
         auto & p = parts->val_str.parts;
-        for (size_t i = 1; i < p.size(); ) {
-            if (p[i].is_input == p[i - 1].is_input) {
-                p[i - 1].val += p[i].val;
-                p.erase(p.begin() + i);
+        if (p.empty()) {
+            return parts;
+        }
+        size_t w = 0;
+        for (size_t r = 1; r < p.size(); r++) {
+            if (p[w].is_input == p[r].is_input) {
+                p[w].val += p[r].val;
             } else {
-                i++;
+                w++;
+                if (w != r) {
+                    // the guard is needed, self-move leaves the string in an unspecified state
+                    p[w] = std::move(p[r]);
+                }
             }
         }
+        p.resize(w + 1);
         return parts;
     }
 

diff --git a/common/jinja/string.cpp b/common/jinja/string.cpp
index 8087e15b350284482d8c95bea6d47291eccd2797..bde679e4e9df92122a9a9c0303cef687835e49df 100644 (file)
--- a/common/jinja/string.cpp
+++ b/common/jinja/string.cpp
@@ -103,7 +103,7 @@ void string::mark_input_based_on(const string & other) {
     }
 }
 
-string string::append(const string & other) {
+string & string::append(const string & other) {
     for (const auto & part : other.parts) {
         parts.push_back(part);
     }

diff --git a/common/jinja/string.h b/common/jinja/string.h
index c4963000adb824fd2b015b3253e50b68e81852ca..669afb8f1da07ea4e246d3c5dfb7829420019c80 100644 (file)
--- a/common/jinja/string.h
+++ b/common/jinja/string.h
@@ -47,7 +47,7 @@ struct string {
     // mark this string as input if other has ALL parts as input
     void mark_input_based_on(const string & other);
 
-    string append(const string & other);
+    string & append(const string & other);
 
     // in-place transformations
 

diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp
index 1ac5b57decca73d102c10cb8a096be53bce12046..d410b48b51407d15c96ebb088f4a52f66b65eb7d 100644 (file)
--- a/tests/test-jinja.cpp
+++ b/tests/test-jinja.cpp
@@ -33,6 +33,7 @@ static void test_array_methods(testing & t);
 static void test_object_methods(testing & t);
 static void test_hasher(testing & t);
 static void test_stats(testing & t);
+static void test_string_parts(testing & t);
 static void test_fuzzing(testing & t);
 
 static bool g_python_mode = false;
@@ -72,6 +73,7 @@ int main(int argc, char *argv[]) {
     if (!g_python_mode) {
         t.test("hasher", test_hasher);
         t.test("stats", test_stats);
+        t.test("string parts", test_string_parts);
         t.test("fuzzing", test_fuzzing);
     }
 
@@ -2057,6 +2059,36 @@ static void test_stats(testing & t) {
     });
 }
 
+static void test_string_parts(testing & t) {
+    static auto render = [](const std::string & tmpl, const json & vars) -> jinja::string {
+        jinja::lexer lexer;
+        auto lexer_res = lexer.tokenize(tmpl);
+
+        jinja::program ast = jinja::parse_from_tokens(lexer_res);
+
+        jinja::context ctx(tmpl);
+        jinja::global_from_json(ctx, vars, true);
+
+        jinja::runtime runtime(ctx);
+        return runtime.gather_string_parts(runtime.execute(ast))->as_string();
+    };
+
+    t.test("merge joins only the neighbours with the same type", [](testing & t) {
+        // "AB" comes from the input and merges, "-" comes from the template and must not
+        jinja::string res = render("{{ val.a }}{{ val.b }}-{{ val.c }}",
+                                   json{{"val", json{{"a", "A"}, {"b", "B"}, {"c", "C"}}}});
+
+        if (t.assert_true("3 parts after the merge", res.parts.size() == 3)) {
+            t.assert_true("part 0 is the merged input", res.parts[0].val == "AB" && res.parts[0].is_input);
+            t.assert_true("part 1 is from the template", res.parts[1].val == "-" && !res.parts[1].is_input);
+            t.assert_true("part 2 is input",             res.parts[2].val == "C" && res.parts[2].is_input);
+        } else {
+            t.log("parts: " + std::to_string(res.parts.size()) + ", rendered: " + json(res.str()).dump());
+        }
+    });
+
+}
+
 static void test_template_cpp(testing & t, const std::string & name, const std::string & tmpl, const json & vars, const std::string & expect) {
     t.test(name, [&tmpl, &vars, &expect](testing & t) {
         jinja::lexer lexer;