Improve memory allocations in execution and storage

Reviewers: teon.banek

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2340

src/communication/bolt/v1/states/executing.hpp

@@ -102,8 +102,8 @@ State HandleRun(TSession &session, State state, Marker marker) {
     std::vector<Value> vec;
     std::map<std::string, Value> data;
     vec.reserve(header.size());
-    for (auto &i : header) vec.push_back(Value(i));
+    for (auto &i : header) vec.emplace_back(std::move(i));
-    data.insert(std::make_pair(std::string("fields"), Value(vec)));
+    data.emplace("fields", std::move(vec));
     // Send the header.
     if (!session.encoder_.MessageSuccess(data)) {
       DLOG(WARNING) << "Couldn't send query header!";

src/glue/communication.cpp

@@ -26,13 +26,13 @@ query::TypedValue ToTypedValue(const Value &value) {
       std::vector<query::TypedValue> list;
       list.reserve(value.ValueList().size());
       for (const auto &v : value.ValueList()) list.push_back(ToTypedValue(v));
-      return query::TypedValue(list);
+      return query::TypedValue(std::move(list));
     }
     case Value::Type::Map: {
       std::map<std::string, query::TypedValue> map;
       for (const auto &kv : value.ValueMap())
         map.emplace(kv.first, ToTypedValue(kv.second));
-      return query::TypedValue(map);
+      return query::TypedValue(std::move(map));
     }
     case Value::Type::Vertex:
     case Value::Type::Edge:
@@ -61,14 +61,14 @@ Value ToBoltValue(const query::TypedValue &value) {
       for (const auto &v : value.ValueList()) {
         values.push_back(ToBoltValue(v));
       }
-      return Value(values);
+      return Value(std::move(values));
     }
     case query::TypedValue::Type::Map: {
       std::map<std::string, Value> map;
       for (const auto &kv : value.ValueMap()) {
         map.emplace(kv.first, ToBoltValue(kv.second));
       }
-      return Value(map);
+      return Value(std::move(map));
     }
     case query::TypedValue::Type::Vertex:
       return Value(ToBoltVertex(value.ValueVertex()));
@@ -91,7 +91,8 @@ communication::bolt::Vertex ToBoltVertex(const VertexAccessor &vertex) {
     properties[vertex.db_accessor().PropertyName(prop.first)] =
         ToBoltValue(prop.second);
   }
-  return communication::bolt::Vertex{id, labels, properties};
+  return communication::bolt::Vertex{id, std::move(labels),
+                                     std::move(properties)};
 }
 
 communication::bolt::Edge ToBoltEdge(const EdgeAccessor &edge) {
@@ -104,7 +105,7 @@ communication::bolt::Edge ToBoltEdge(const EdgeAccessor &edge) {
     properties[edge.db_accessor().PropertyName(prop.first)] =
         ToBoltValue(prop.second);
   }
-  return communication::bolt::Edge{id, from, to, type, properties};
+  return communication::bolt::Edge{id, from, to, type, std::move(properties)};
 }
 
 communication::bolt::Path ToBoltPath(const query::Path &path) {
@@ -175,7 +176,7 @@ Value ToBoltValue(const PropertyValue &value) {
       for (const auto &v : values) {
         vec.push_back(ToBoltValue(v));
       }
-      return Value(vec);
+      return Value(std::move(vec));
     }
     case PropertyValue::Type::Map: {
       const auto &map = value.ValueMap();
@@ -183,7 +184,7 @@ Value ToBoltValue(const PropertyValue &value) {
       for (const auto &kv : map) {
         dv_map.emplace(kv.first, ToBoltValue(kv.second));
       }
-      return Value(dv_map);
+      return Value(std::move(dv_map));
     }
   }
 }

src/query/interpreter.cpp

@@ -831,8 +831,9 @@ Interpreter::Results Interpreter::operator()(
                            .first);
     }
 
-    return Results(&db_accessor, parameters, plan, output_symbols, header,
+    return Results(&db_accessor, parameters, plan, std::move(output_symbols),
-                   summary, parsed_query.required_privileges, execution_memory);
+                   std::move(header), std::move(summary),
+                   parsed_query.required_privileges, execution_memory);
   }
 
   if (utils::IsSubtype(*parsed_query.query, ExplainQuery::kType)) {
@@ -901,8 +902,9 @@ Interpreter::Results Interpreter::operator()(
 
     std::vector<std::string> header{query_plan_symbol.name()};
 
-    return Results(&db_accessor, parameters, plan, output_symbols, header,
+    return Results(&db_accessor, parameters, plan, std::move(output_symbols),
-                   summary, parsed_query.required_privileges, execution_memory);
+                   std::move(header), std::move(summary),
+                   parsed_query.required_privileges, execution_memory);
   }
 
   if (utils::IsSubtype(*parsed_query.query, ProfileQuery::kType)) {
@@ -982,8 +984,9 @@ Interpreter::Results Interpreter::operator()(
     auto planning_time = planning_timer.Elapsed();
     summary["planning_time"] = planning_time.count();
 
-    return Results(&db_accessor, parameters, plan, output_symbols, header,
+    return Results(&db_accessor, parameters, plan, std::move(output_symbols),
-                   summary, parsed_query.required_privileges, execution_memory,
+                   std::move(header), std::move(summary),
+                   parsed_query.required_privileges, execution_memory,
                    /* is_profile_query */ true, /* should_abort_query */ true);
   }
 
@@ -1004,8 +1007,9 @@ Interpreter::Results Interpreter::operator()(
 
     summary["planning_time"] = planning_timer.Elapsed().count();
 
-    return Results(&db_accessor, parameters, plan, output_symbols, header,
+    return Results(&db_accessor, parameters, plan, std::move(output_symbols),
-                   summary, parsed_query.required_privileges, execution_memory,
+                   std::move(header), std::move(summary),
+                   parsed_query.required_privileges, execution_memory,
                    /* is_profile_query */ false,
                    /* should_abort_query */ false);
 #else
@@ -1077,9 +1081,9 @@ Interpreter::Results Interpreter::operator()(
   summary["planning_time"] = planning_time.count();
   summary["cost_estimate"] = 0.0;
 
-  return Results(&db_accessor, parameters, plan, output_symbols,
+  return Results(&db_accessor, parameters, plan, std::move(output_symbols),
-                 callback.header, summary, parsed_query.required_privileges,
+                 callback.header, std::move(summary),
-                 execution_memory,
+                 parsed_query.required_privileges, execution_memory,
                  /* is_profile_query */ false, callback.should_abort_query);
 }
 

src/query/interpreter.hpp

@@ -94,9 +94,9 @@ class Interpreter {
           plan_(plan),
           cursor_(plan_->plan().MakeCursor(execution_memory)),
           frame_(plan_->symbol_table().max_position(), execution_memory),
-          output_symbols_(output_symbols),
+          output_symbols_(std::move(output_symbols)),
-          header_(header),
+          header_(std::move(header)),
-          summary_(summary),
+          summary_(std::move(summary)),
           privileges_(std::move(privileges)),
           should_abort_query_(should_abort_query) {
       ctx_.is_profile_query = is_profile_query;
@@ -172,8 +172,14 @@ class Interpreter {
       while (Pull(stream)) continue;
     }
 
-    const std::vector<std::string> &header() { return header_; }
+    const std::vector<std::string> &header() const & { return header_; }
-    const std::map<std::string, TypedValue> &summary() { return summary_; }
+    std::vector<std::string> &&header() && { return std::move(header_); }
+    const std::map<std::string, TypedValue> &summary() const & {
+      return summary_;
+    }
+    std::map<std::string, TypedValue> &&summary() && {
+      return std::move(summary_);
+    }
 
     const std::vector<AuthQuery::Privilege> &privileges() {
       return privileges_;

src/query/transaction_engine.hpp

@@ -83,7 +83,7 @@ class TransactionEngine final {
       results_.emplace((*interpreter_)(query, *db_accessor_, params,
                                        in_explicit_transaction_,
                                        &execution_memory_));
-      return {results_->header(), results_->privileges()};
+      return {std::move(results_->header()), std::move(results_->privileges())};
     } catch (const utils::BasicException &) {
       AbortCommand();
       throw;

src/storage/v2/name_id_mapper.hpp

@@ -52,7 +52,11 @@ class NameIdMapper final {
     // We have to try to insert the ID to name mapping even if we are not the
     // one who assigned the ID because we have to make sure that after this
     // method returns that both mappings exist.
-    id_to_name_acc.insert({id, name});
+    if (id_to_name_acc.find(id) == id_to_name_acc.end()) {
+      // We first try to find the `id` in the map to avoid making an unnecessary
+      // temporary memory allocation when the object already exists.
+      id_to_name_acc.insert({id, name});
+    }
     return id;
   }