Merge remote-tracking branch 'origin/topic/bbannier/issue-1742'

CHANGES

@@ -1,3 +1,20 @@
+1.11.0-dev.172 | 2024-05-17 12:51:34 +0200
+
+  * GH-1742: Unroll ctrs of big containers. (Benjamin Bannier, Corelight)
+
+    When generating C++ code for container ctrs we previously would directly
+    invoke the respective C++ ctrs taking an initializer list. For very big
+    initializer lists this causes very bad C++ compiler performance, e.g.,
+    compiling code constructing a vector with 10,000 elements could take
+    minutes.
+
+    With this patch we unroll such ctrs calls by calling a dedicated
+    initialization function. For huge containers this causes creating of big
+    functions instead of big initializer lists, but compiling functions
+    seems to behave more predictively.
+
+    Closes #1742.
+
 1.11.0-dev.170 | 2024-05-17 12:51:06 +0200
 
   * GH-1743: Use a checked cast for `map`'s `in` operator. (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-1.11.0-dev.170
+1.11.0-dev.172

hilti/toolchain/src/compiler/codegen/ctors.cc

@@ -17,6 +17,11 @@ using hilti::rt::fmt;
 
 namespace {
 
+// The container size threshold over which we unroll container ctrs. This is to
+// avoid running into C++ compilation performance edge cases for container
+// construction when passing huge initializer lists.
+constexpr auto ThresholdBigContainerCtrUnroll = 10U;
+
 struct Visitor : hilti::visitor::PreOrder {
     explicit Visitor(CodeGen* cg) : cg(cg) {}
 
@@ -103,13 +108,25 @@ struct Visitor : hilti::visitor::PreOrder {
         auto k = cg->compile(n->keyType(), codegen::TypeUsage::Storage);
         auto v = cg->compile(n->valueType(), codegen::TypeUsage::Storage);
 
-        result =
-            fmt("::hilti::rt::Map<%s, %s>({%s})", k, v,
-                util::join(node::transform(n->value(),
-                                           [this](const auto& e) {
-                                               return fmt("{%s, %s}", cg->compile(e->key()), cg->compile(e->value()));
-                                           }),
-                           ", "));
+        if ( const auto size = n->value().size(); size > ThresholdBigContainerCtrUnroll ) {
+            auto elems = util::join(node::transform(n->value(),
+                                                    [this](const auto& e) {
+                                                        return fmt("__xs.index_assign(%s, %s);", cg->compile(e->key()),
+                                                                   cg->compile(e->value()));
+                                                    }),
+                                    " ");
+
+            result = fmt("[&]() { auto __xs = ::hilti::rt::Map<%s, %s>(); %s return __xs; }()", k, v, elems);
+        }
+
+        else
+            result = fmt("::hilti::rt::Map<%s, %s>({%s})", k, v,
+                         util::join(node::transform(n->value(),
+                                                    [this](const auto& e) {
+                                                        return fmt("{%s, %s}", cg->compile(e->key()),
+                                                                   cg->compile(e->value()));
+                                                    }),
+                                    ", "));
     }
 
     void operator()(ctor::Network* n) final {
@@ -176,9 +193,20 @@ struct Visitor : hilti::visitor::PreOrder {
 
         const auto k = cg->compile(n->elementType(), codegen::TypeUsage::Storage);
 
-        result =
-            fmt("::hilti::rt::Set<%s>({%s})", k,
-                util::join(node::transform(n->value(), [this](auto e) { return fmt("%s", cg->compile(e)); }), ", "));
+        if ( const auto size = n->value().size(); size > ThresholdBigContainerCtrUnroll ) {
+            auto elems =
+                util::join(node::transform(n->value(),
+                                           [this](const auto& e) { return fmt("__xs.insert(%s);", cg->compile(e)); }),
+                           " ");
+
+            result = fmt("[&]() { auto __xs = ::hilti::rt::Set<%s>(); %s return __xs; }()", k, elems);
+        }
+
+        else
+            result =
+                fmt("::hilti::rt::Set<%s>({%s})", k,
+                    util::join(node::transform(n->value(), [this](const auto& e) { return fmt("%s", cg->compile(e)); }),
+                               ", "));
     }
 
     void operator()(ctor::SignedInteger* n) final {
@@ -252,9 +280,22 @@ struct Visitor : hilti::visitor::PreOrder {
         if ( auto def = cg->typeDefaultValue(n->elementType()) )
             allocator = fmt(", hilti::rt::vector::Allocator<%s, %s>", x, *def);
 
-        result =
-            fmt("::hilti::rt::Vector<%s%s>({%s})", x, allocator,
-                util::join(node::transform(n->value(), [this](auto e) { return fmt("%s", cg->compile(e)); }), ", "));
+        if ( const auto size = n->value().size(); size > ThresholdBigContainerCtrUnroll ) {
+            auto elems = util::join(node::transform(n->value(),
+                                                    [this](const auto& e) {
+                                                        return fmt("__xs.push_back(%s);", cg->compile(e));
+                                                    }),
+                                    " ");
+
+            result = fmt("[&]() { auto __xs = ::hilti::rt::Vector<%s%s>(); __xs.reserve(%d); %s return __xs; }()", x,
+                         allocator, size, elems);
+        }
+
+        else
+            result =
+                fmt("::hilti::rt::Vector<%s%s>({%s})", x, allocator,
+                    util::join(node::transform(n->value(), [this](const auto& e) { return fmt("%s", cg->compile(e)); }),
+                               ", "));
     }
 
     void operator()(ctor::UnsignedInteger* n) final {

tests/Baseline/hilti.types.map.ctor/output

@@ -1 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+{0: 0, 1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8, 9: 9, 10: 10, 11: 11, 12: 12, 13: 13, 14: 14, 15: 15, 16: 16, 17: 17, 18: 18, 19: 19}

tests/Baseline/hilti.types.set.ctor/output

@@ -1 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+{0, 1, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 2, 3, 4, 5, 6, 7, 8, 9}

tests/Baseline/hilti.types.vector.ctor/output

@@ -14,3 +14,4 @@
 ["s1", "s2"]
 [[1, 2], [3, 4], [4, 5]]
 [True, False, True, False]
+[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

tests/hilti/types/map/ctor.hlt

@@ -3,6 +3,8 @@
 
 module Test {
 
+import hilti;
+
 global value_ref<string> x = "foo"; # type that coerces to string
 
 assert map("foo": "1", "2": "foo") == map<string, string>(x: "1", "2": x);
@@ -11,4 +13,30 @@ global auto i = 1;
 global auto j = -1;
 assert map(i: j) == map(1:-1);
 assert map<uint<64>, int<64>>(i: j) == map(1:-1);
+
+# Test unrolling of ctor for big map ctrs.
+global m = map(
+    0: 0,
+    1: 1,
+    2: 2,
+    3: 3,
+    4: 4,
+    5: 5,
+    6: 6,
+    7: 7,
+    8: 8,
+    9: 9,
+    10: 10,
+    11: 11,
+    12: 12,
+    13: 13,
+    14: 14,
+    15: 15,
+    16: 16,
+    17: 17,
+    18: 18,
+    19: 19
+);
+hilti::print(m);
+
 }

tests/hilti/types/set/ctor.hlt

@@ -3,11 +3,18 @@
 
 module Test {
 
+import hilti;
+
 global value_ref<string> x = "foo"; # type that coerces to string
 assert set("foo", "bar") == set<string>(x, "bar");
 
 global auto i = 1;
 global auto j = 2;
 assert set(i, j) == set(1, 2);
 assert set<uint<64>>(i, j) == set(1, 2);
+
+# Test unrolling of ctor for big set ctrs.
+global s = set(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19);
+hilti::print(s);
+
 }

tests/hilti/types/vector/ctor.hlt

@@ -45,4 +45,8 @@ hilti::print(l6);
 global value_ref<string> x = "foo"; # type that coerces to string
 assert vector("foo", "bar") == vector<string>(x, "bar");
 
+# Test unrolling of ctor for big vector ctrs.
+global x10 = vector(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9);
+hilti::print(x10);
+
 }