Merge branch 'MG-benchmark-pullmultiple-expand' into MG-benchmark-expand

src/query/v2/interpreter.cpp

@@ -64,11 +64,7 @@
 #include "utils/tsc.hpp"
 #include "utils/variant_helpers.hpp"
 
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-DEFINE_bool(use_multi_frame, false, "Whether to use MultiFrame or not");
-
 namespace EventCounter {
-
 extern Event ReadQuery;
 extern Event WriteQuery;
 extern Event ReadWriteQuery;
@@ -78,7 +74,6 @@ extern const Event LabelPropertyIndexCreated;
 
 extern const Event StreamsCreated;
 extern const Event TriggersCreated;
-
 }  // namespace EventCounter
 
 namespace memgraph::query::v2 {
@@ -693,7 +688,7 @@ PullPlan::PullPlan(const std::shared_ptr<CachedPlan> plan, const Parameters &par
     : plan_(plan),
       cursor_(plan->plan().MakeCursor(execution_memory)),
       frame_(plan->symbol_table().max_position(), execution_memory),
-      multi_frame_(plan->symbol_table().max_position(), FLAGS_default_multi_frame_size, execution_memory),
+      multi_frame_(plan->symbol_table().max_position(), kNumberOfFramesInMultiframe, execution_memory),
       memory_limit_(memory_limit) {
   ctx_.db_accessor = dba;
   ctx_.symbol_table = plan->symbol_table();
@@ -817,7 +812,8 @@ std::optional<plan::ProfilingStatsWithTotalTime> PullPlan::PullMultiple(AnyStrea
 std::optional<plan::ProfilingStatsWithTotalTime> PullPlan::Pull(AnyStream *stream, std::optional<int> n,
                                                                 const std::vector<Symbol> &output_symbols,
                                                                 std::map<std::string, TypedValue> *summary) {
-  if (FLAGS_use_multi_frame) {
+  auto should_pull_multiple = true;  // TODO on the long term, we will only use PullMultiple
+  if (should_pull_multiple) {
     return PullMultiple(stream, n, output_symbols, summary);
   }
   // Set up temporary memory for a single Pull. Initial memory comes from the

src/query/v2/multiframe.cpp

@@ -17,9 +17,6 @@
 #include "query/v2/bindings/frame.hpp"
 #include "utils/pmr/vector.hpp"
 
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-DEFINE_uint64(default_multi_frame_size, 100, "Default size of MultiFrame");
-
 namespace memgraph::query::v2 {
 
 static_assert(std::forward_iterator<ValidFramesReader::Iterator>);

src/query/v2/multiframe.hpp

@@ -13,14 +13,10 @@
 
 #include <iterator>
 
-#include <gflags/gflags.h>
-
 #include "query/v2/bindings/frame.hpp"
 
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-DECLARE_uint64(default_multi_frame_size);
-
 namespace memgraph::query::v2 {
+constexpr uint64_t kNumberOfFramesInMultiframe = 1000;  // TODO have it configurable
 
 class ValidFramesConsumer;
 class ValidFramesModifier;

src/query/v2/plan/operator.cpp

@@ -509,7 +509,7 @@ class DistributedScanAllAndFilterCursor : public Cursor {
 
     if (!own_multi_frame_.has_value()) {
       own_multi_frame_.emplace(MultiFrame(output_multi_frame.GetFirstFrame().elems().size(),
-                                          FLAGS_default_multi_frame_size, output_multi_frame.GetMemoryResource()));
+                                          kNumberOfFramesInMultiframe, output_multi_frame.GetMemoryResource()));
       own_frames_consumer_.emplace(own_multi_frame_->GetValidFramesConsumer());
       own_frames_it_ = own_frames_consumer_->begin();
     }
@@ -705,7 +705,7 @@ class DistributedScanByPrimaryKeyCursor : public Cursor {
   void EnsureOwnMultiFrameIsGood(MultiFrame &output_multi_frame) {
     if (!own_multi_frame_.has_value()) {
       own_multi_frame_.emplace(MultiFrame(output_multi_frame.GetFirstFrame().elems().size(),
-                                          FLAGS_default_multi_frame_size, output_multi_frame.GetMemoryResource()));
+                                          kNumberOfFramesInMultiframe, output_multi_frame.GetMemoryResource()));
       own_frames_consumer_.emplace(own_multi_frame_->GetValidFramesConsumer());
       own_frames_it_ = own_frames_consumer_->begin();
     }
@@ -1347,14 +1347,15 @@ bool ContainsSameEdge(const TypedValue &a, const TypedValue &b) {
 }
 
 bool IsExpansionOk(Frame &frame, const Symbol &expand_symbol, const std::vector<Symbol> &previous_symbols) {
-  // This shouldn't raise a TypedValueException, because the planner
-  // makes sure these are all of the expected type. In case they are not
-  // an error should be raised long before this code is executed.
-  return std::ranges::all_of(previous_symbols,
-                             [&frame, &expand_value = frame[expand_symbol]](const auto &previous_symbol) {
-                               const auto &previous_value = frame[previous_symbol];
-                               return !ContainsSameEdge(previous_value, expand_value);
-                             });
+  const auto &expand_value = frame[expand_symbol];
+  for (const auto &previous_symbol : previous_symbols) {
+    const auto &previous_value = frame[previous_symbol];
+    // This shouldn't raise a TypedValueException, because the planner
+    // makes sure these are all of the expected type. In case they are not
+    // an error should be raised long before this code is executed.
+    if (ContainsSameEdge(previous_value, expand_value)) return false;
+  }
+  return true;
 }
 
 }  // namespace
@@ -2213,7 +2214,7 @@ class UnwindCursor : public Cursor {
 
     if (!own_multi_frame_.has_value()) {
       own_multi_frame_.emplace(MultiFrame(output_multi_frame.GetFirstFrame().elems().size(),
-                                          FLAGS_default_multi_frame_size, output_multi_frame.GetMemoryResource()));
+                                          kNumberOfFramesInMultiframe, output_multi_frame.GetMemoryResource()));
       own_frames_consumer_.emplace(own_multi_frame_->GetValidFramesConsumer());
       own_frames_it_ = own_frames_consumer_->begin();
     }
@@ -3382,7 +3383,7 @@ class DistributedExpandCursor : public Cursor {
   void EnsureOwnMultiFrameIsGood(MultiFrame &output_multi_frame) {
     if (!own_multi_frame_.has_value()) {
       own_multi_frame_.emplace(MultiFrame(output_multi_frame.GetFirstFrame().elems().size(),
-                                          FLAGS_default_multi_frame_size, output_multi_frame.GetMemoryResource()));
+                                          kNumberOfFramesInMultiframe, output_multi_frame.GetMemoryResource()));
       own_frames_consumer_.emplace(own_multi_frame_->GetValidFramesConsumer());
       own_frames_it_ = own_frames_consumer_->begin();
     }

tests/mgbench/compare_results.py

@@ -14,6 +14,7 @@
 import argparse
 import json
 
+
 FIELDS = [
     {
         "name": "throughput",
@@ -84,32 +85,39 @@ def compare_results(results_from, results_to, fields):
                 if group == "__import__":
                     continue
                 for scenario, summary_to in scenarios.items():
-                    summary_from = recursive_get(results_from, dataset, variant, group, scenario, value={})
-                    if (
-                        len(summary_from) > 0
-                        and summary_to["count"] != summary_from["count"]
-                        or summary_to["num_workers"] != summary_from["num_workers"]
-                    ):
+                    summary_from = recursive_get(
+                        results_from, dataset, variant, group, scenario,
+                        value={})
+                    if len(summary_from) > 0 and \
+                            summary_to["count"] != summary_from["count"] or \
+                            summary_to["num_workers"] != \
+                            summary_from["num_workers"]:
                         raise Exception("Incompatible results!")
-                    testcode = "/".join([dataset, variant, group, scenario, "{:02d}".format(summary_to["num_workers"])])
+                    testcode = "/".join([dataset, variant, group, scenario,
+                                         "{:02d}".format(
+                                             summary_to["num_workers"])])
                     row = {}
                     performance_changed = False
                     for field in fields:
                         key = field["name"]
                         if key in summary_to:
-                            row[key] = compute_diff(summary_from.get(key, None), summary_to[key])
+                            row[key] = compute_diff(
+                                summary_from.get(key, None),
+                                summary_to[key])
                         elif key in summary_to["database"]:
                             row[key] = compute_diff(
-                                recursive_get(summary_from, "database", key, value=None), summary_to["database"][key]
-                            )
+                                recursive_get(summary_from, "database", key,
+                                              value=None),
+                                summary_to["database"][key])
                         else:
                             row[key] = compute_diff(
-                                recursive_get(summary_from, "metadata", key, "average", value=None),
-                                summary_to["metadata"][key]["average"],
-                            )
-                        if "diff" not in row[key] or (
-                            "diff_treshold" in field and abs(row[key]["diff"]) >= field["diff_treshold"]
-                        ):
+                                recursive_get(summary_from, "metadata", key,
+                                              "average", value=None),
+                                summary_to["metadata"][key]["average"])
+                        if "diff" not in row[key] or \
+                                ("diff_treshold" in field and
+                                 abs(row[key]["diff"]) >=
+                                 field["diff_treshold"]):
                             performance_changed = True
                     if performance_changed:
                         ret[testcode] = row
@@ -122,36 +130,29 @@ def generate_remarkup(fields, data):
         ret += "<table>\n"
         ret += "  <tr>\n"
         ret += "    <th>Testcode</th>\n"
-        ret += (
-            "\n".join(
-                map(
-                    lambda x: "    <th>{}</th>".format(x["name"].replace("_", " ").capitalize()),
-                    fields,
-                )
-            )
-            + "\n"
-        )
+        ret += "\n".join(map(lambda x: "    <th>{}</th>".format(
+            x["name"].replace("_", " ").capitalize()), fields)) + "\n"
         ret += "  </tr>\n"
         for testcode in sorted(data.keys()):
             ret += "  <tr>\n"
             ret += "    <td>{}</td>\n".format(testcode)
             for field in fields:
-                result = data[testcode].get(field["name"])
-                if result != None:
-                    value = result["value"] * field["scaling"]
-                    if "diff" in result:
-                        diff = result["diff"]
-                        arrow = "arrow-up" if diff >= 0 else "arrow-down"
-                        if not (field["positive_diff_better"] ^ (diff >= 0)):
-                            color = "green"
-                        else:
-                            color = "red"
-                        sign = "{{icon {} color={}}}".format(arrow, color)
-                        ret += '    <td bgcolor="{}">{:.3f}{} ({:+.2%})</td>\n'.format(
-                            color, value, field["unit"], diff
-                        )
+                result = data[testcode][field["name"]]
+                value = result["value"] * field["scaling"]
+                if "diff" in result:
+                    diff = result["diff"]
+                    arrow = "arrow-up" if diff >= 0 else "arrow-down"
+                    if not (field["positive_diff_better"] ^ (diff >= 0)):
+                        color = "green"
                     else:
-                        ret += '<td bgcolor="blue">{:.3f}{} //(new)// </td>\n'.format(value, field["unit"])
+                        color = "red"
+                    sign = "{{icon {} color={}}}".format(arrow, color)
+                    ret += "    <td>{:.3f}{} //({:+.2%})// {}</td>\n".format(
+                        value, field["unit"], diff, sign)
+                else:
+                    ret += "    <td>{:.3f}{} //(new)// " \
+                           "{{icon plus color=blue}}</td>\n".format(
+                               value, field["unit"])
             ret += "  </tr>\n"
         ret += "</table>\n"
     else:
@@ -160,14 +161,11 @@ def generate_remarkup(fields, data):
 
 
 if __name__ == "__main__":
-    parser = argparse.ArgumentParser(description="Compare results of multiple benchmark runs.")
-    parser.add_argument(
-        "--compare",
-        action="append",
-        nargs=2,
-        metavar=("from", "to"),
-        help="compare results between `from` and `to` files",
-    )
+    parser = argparse.ArgumentParser(
+        description="Compare results of multiple benchmark runs.")
+    parser.add_argument("--compare", action="append", nargs=2,
+                        metavar=("from", "to"),
+                        help="compare results between `from` and `to` files")
     parser.add_argument("--output", default="", help="output file name")
     args = parser.parse_args()
 

tests/mgbench/dataset_creator_unwind.py

@@ -55,6 +55,7 @@ def main():
     parser.add_argument("--number_of_files", type=int, default=10)
     parser.add_argument("--percentage_of_permissions", type=float, default=1.0)
     parser.add_argument("--filename", default="dataset.cypher")
+    parser.add_argument("--expand_filename", default="expand_output.txt")
 
     args = parser.parse_args()
 
@@ -62,6 +63,7 @@ def main():
     number_of_files = args.number_of_files
     percentage_of_permissions = args.percentage_of_permissions
     filename = args.filename
+    expand_filename = args.expand_filename
 
     assert number_of_identities >= 0
     assert number_of_files >= 0
@@ -106,33 +108,48 @@ def main():
             uuid += 1
         f.write("] AS props CREATE (:Identity {uuid: props.uuid, name: props.name});\n")
 
-        f.write("UNWIND [")
-        created = 0
-        for outer_index in range(0, number_of_files):
-            for inner_index in range(0, number_of_identities):
+        with open(expand_filename, "w") as ef:
 
-                file_uuid = outer_index + 1
-                identity_uuid = number_of_files + inner_index + 1
+            f.write("UNWIND [")
+            ef.write("UNWIND [")
 
-                if random.random() <= percentage_of_permissions:
+            created = 0
+            for outer_index in range(0, number_of_files):
+                for inner_index in range(0, number_of_identities):
 
-                    if created > 0:
-                        f.write(",")
+                    file_uuid = outer_index + 1
+                    identity_uuid = number_of_files + inner_index + 1
 
-                    f.write(
-                        f' {{permUuid: {uuid}, permName: "name_permission_{uuid}", fileUuid: {file_uuid}, identityUuid: {identity_uuid}}}'
-                    )
-                    created += 1
-                    uuid += 1
+                    if random.random() <= percentage_of_permissions:
 
-                    if created == 5000:
-                        f.write(
-                            "] AS props MATCH (file:File {uuid:props.fileUuid}), (identity:Identity {uuid: props.identityUuid}) CREATE (permission:Permission {uuid: props.permUuid, name: props.permName}) CREATE (permission)-[: IS_FOR_FILE]->(file) CREATE (permission)-[: IS_FOR_IDENTITY]->(identity);\nUNWIND ["
+                        if created > 0:
+                            ef.write(",")
+                            f.write(",")
+
+                        ef.write(
+                            f' {{permUuid: {uuid}, permName: "name_permission_{uuid}", fileUuid: {file_uuid}, identityUuid: {identity_uuid}}}'
                         )
-                        created = 0
-        f.write(
-            "] AS props MATCH (file:File {uuid:props.fileUuid}), (identity:Identity {uuid: props.identityUuid}) CREATE (permission:Permission {uuid: props.permUuid, name: props.permName}) CREATE (permission)-[: IS_FOR_FILE]->(file) CREATE (permission)-[: IS_FOR_IDENTITY]->(identity);\n"
-        )
+
+                        f.write(
+                            f' {{permUuid: {uuid}, permName: "name_permission_{uuid}", fileUuid: {file_uuid}, identityUuid: {identity_uuid}}}'
+                        )
+                        created += 1
+                        uuid += 1
+
+                        if created == 5000:
+                            ef.write(
+                                "] AS props MATCH (permission:Permission {uuid: props.permUuid, name: props.permName})-[: IS_FOR_FILE]->(file:File {uuid:props.fileUuid}), (permission:Permission {uuid: props.permUuid, name: props.permName})-[: IS_FOR_IDENTITY]->(identity:Identity {uuid: props.identityUuid}) RETURN permission, file, identity;\nUNWIND ["
+                            )
+                            f.write(
+                                "] AS props MATCH (file:File {uuid:props.fileUuid}), (identity:Identity {uuid: props.identityUuid}) CREATE (permission:Permission {uuid: props.permUuid, name: props.permName}) CREATE (permission)-[: IS_FOR_FILE]->(file) CREATE (permission)-[: IS_FOR_IDENTITY]->(identity);\nUNWIND ["
+                            )
+                            created = 0
+            ef.write(
+                "] AS props MATCH (permission:Permission {uuid: props.permUuid, name: props.permName})-[: IS_FOR_FILE]->(file:File {uuid:props.fileUuid}), (permission:Permission {uuid: props.permUuid, name: props.permName})-[: IS_FOR_IDENTITY]->(identity:Identity {uuid: props.identityUuid}) RETURN permission, file, identity;"
+            )
+            f.write(
+                "] AS props MATCH (file:File {uuid:props.fileUuid}), (identity:Identity {uuid: props.identityUuid}) CREATE (permission:Permission {uuid: props.permUuid, name: props.permName}) CREATE (permission)-[: IS_FOR_FILE]->(file) CREATE (permission)-[: IS_FOR_IDENTITY]->(identity);"
+            )
 
 
 if __name__ == "__main__":

tests/mgbench/datasets.py

@@ -353,7 +353,7 @@ class AccessControl(Dataset):
 
     def benchmark__create__vertex(self):
         self.next_value_idx += 1
-        query = ("CREATE (:File {uuid: $uuid})", {"uuid": self.next_value_idx})
+        query = (f"CREATE (:File {{uuid: {self.next_value_idx}}});", {})
         return query
 
     def benchmark__create__edges(self):
@@ -379,24 +379,6 @@ class AccessControl(Dataset):
         return query
 
     def benchmark__match__match_all_vertices_with_edges(self):
+        self.next_value_idx += 1
         query = ("MATCH (permission:Permission)-[e:IS_FOR_FILE]->(file:File) RETURN *", {})
         return query
-
-    def benchmark__match__match_users_with_permission_for_files(self):
-        file_uuid_1 = self._get_random_uuid("File")
-        file_uuid_2 = self._get_random_uuid("File")
-        min_file_uuid = min(file_uuid_1, file_uuid_2)
-        max_file_uuid = max(file_uuid_1, file_uuid_2)
-        query = (
-            "MATCH (f:File)<-[ff:IS_FOR_FILE]-(p:Permission)-[fi:IS_FOR_IDENTITY]->(i:Identity) WHERE f.uuid >= $min_file_uuid  AND f.uuid <= $max_file_uuid RETURN *",
-            {"min_file_uuid": min_file_uuid, "max_file_uuid": max_file_uuid},
-        )
-        return query
-
-    def benchmark__match__match_users_with_permission_for_specific_file(self):
-        file_uuid = self._get_random_uuid("File")
-        query = (
-            "MATCH (f:File {uuid: $file_uuid})<-[ff:IS_FOR_FILE]-(p:Permission)-[fi:IS_FOR_IDENTITY]->(i:Identity) RETURN *",
-            {"file_uuid": file_uuid},
-        )
-        return query

tests/mgbench/runners.py

@@ -68,15 +68,6 @@ class Memgraph:
         self._cleanup()
         atexit.unregister(self._cleanup)
 
-    # Returns None if string_value is not true or false, casing doesn't matter
-    def _get_bool_value(self, string_value):
-        lower_string_value = string_value.lower()
-        if lower_string_value == "true":
-            return True
-        if lower_string_value == "false":
-            return False
-        return None
-
     def _get_args(self, **kwargs):
         data_directory = os.path.join(self._directory.name, "memgraph")
         if self._memgraph_version >= (0, 50, 0):
@@ -92,13 +83,7 @@ class Memgraph:
             args_list = self._extra_args.split(" ")
             assert len(args_list) % 2 == 0
             for i in range(0, len(args_list), 2):
-                key = args_list[i]
-                value = args_list[i + 1]
-                maybe_bool_value = self._get_bool_value(value)
-                if maybe_bool_value is not None:
-                    kwargs[key] = maybe_bool_value
-                else:
-                    kwargs[key] = value
+                kwargs[args_list[i]] = args_list[i + 1]
 
         return _convert_args_to_flags(self._memgraph_binary, **kwargs)
 

tools/plot/.gitignore

@@ -1,2 +0,0 @@
-*.dat
-*.out

tools/plot/pool_size_batch_size_query_latency.gnuplot

@@ -1,7 +0,0 @@
-set dgrid3d 30,30
-set hidden3d
-set label "pool size"   at 8, 11000, 0
-set label "multiframe size"  at 20,6000,0
-set label "execution time (ms)"  at 14,0,5000
-splot "frames1.dat" u 1:2:3 with lines
-pause mouse close

tools/plot/thread_shard_scanall_latency.gnuplot

@@ -1,6 +0,0 @@
-set dgrid3d 30,30
-set hidden3d
-set label "shards"   at 20,-1,10000
-set label "threads"  at -10,5,10000
-splot "data.dat" u 1:2:3 with lines
-pause mouse close