Use new SkipList for query caches

Reviewers: teon.banek, mtomic

Reviewed By: teon.banek

Subscribers: pullbot

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

src/query/interpreter.cpp

@@ -1095,9 +1095,9 @@ std::shared_ptr<Interpreter::CachedPlan> Interpreter::CypherQueryToPlan(
     }
   }
   return plan_cache_access
-      .insert(query_hash,
+      .insert({query_hash,
                std::make_shared<CachedPlan>(MakeLogicalPlan(
-                  query, std::move(ast_storage), parameters, db_accessor)))
+                   query, std::move(ast_storage), parameters, db_accessor))})
       .first->second;
 }
 
@@ -1148,8 +1148,8 @@ Interpreter::ParsedQuery Interpreter::ParseQuery(
     CachedQuery cached_query{std::move(cached_ast_storage), visitor.query(),
                              query::GetRequiredPrivileges(visitor.query())};
     // Cache it.
-    ast_it =
+    ast_it = ast_cache_accessor
-        ast_cache_accessor.insert(stripped_query_hash, std::move(cached_query))
+                 .insert({stripped_query_hash, std::move(cached_query)})
                  .first;
   }
   ast_storage->properties_ = ast_it->second.ast_storage.properties_;

src/query/interpreter.hpp

@@ -2,7 +2,6 @@
 
 #include <gflags/gflags.h>
 
-#include "data_structures/concurrent/concurrent_map.hpp"
 #include "database/graph_db.hpp"
 #include "database/graph_db_accessor.hpp"
 #include "query/context.hpp"
@@ -11,6 +10,7 @@
 #include "query/frontend/stripped.hpp"
 #include "query/interpret/frame.hpp"
 #include "query/plan/operator.hpp"
+#include "utils/skip_list.hpp"
 #include "utils/spin_lock.hpp"
 #include "utils/timer.hpp"
 
@@ -66,8 +66,6 @@ class Interpreter {
     std::vector<AuthQuery::Privilege> required_privileges;
   };
 
-  using PlanCacheT = ConcurrentMap<HashType, std::shared_ptr<CachedPlan>>;
-
  public:
   /**
    * Wraps a `Query` that was created as a result of parsing a query string
@@ -244,8 +242,41 @@ class Interpreter {
                                  const plan::LogicalOperator *);
 
  private:
-  ConcurrentMap<HashType, CachedQuery> ast_cache_;
+  struct QueryCacheEntry {
-  PlanCacheT plan_cache_;
+    bool operator==(const QueryCacheEntry &other) const {
+      return first == other.first;
+    }
+    bool operator<(const QueryCacheEntry &other) const {
+      return first < other.first;
+    }
+    bool operator==(const HashType &other) const { return first == other; }
+    bool operator<(const HashType &other) const { return first < other; }
+
+    HashType first;
+    // TODO: Maybe store the query string here and use it as a key with the hash
+    // so that we eliminate the risk of hash collisions.
+    CachedQuery second;
+  };
+
+  struct PlanCacheEntry {
+    bool operator==(const PlanCacheEntry &other) const {
+      return first == other.first;
+    }
+    bool operator<(const PlanCacheEntry &other) const {
+      return first < other.first;
+    }
+    bool operator==(const HashType &other) const { return first == other; }
+    bool operator<(const HashType &other) const { return first < other; }
+
+    HashType first;
+    // TODO: Maybe store the query string here and use it as a key with the hash
+    // so that we eliminate the risk of hash collisions.
+    std::shared_ptr<CachedPlan> second;
+  };
+
+  utils::SkipList<QueryCacheEntry> ast_cache_;
+  utils::SkipList<PlanCacheEntry> plan_cache_;
+
   // Antlr has singleton instance that is shared between threads. It is
   // protected by locks inside of antlr. Unfortunately, they are not protected
   // in a very good way. Once we have antlr version without race conditions we