GDV-31:[Java][C++]Fixed concurrency issue in cache. (apache#84)

* GDV-31:[Java][C++]Fixed concurrency issue in cache. Modifications were happening in get without a mutex. Wrote a test to verify and prevent regression.
praveenbingo · Sep 10, 2018 · 49f6af5 · 49f6af5
1 parent 3c644f3
commit 49f6af5
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 10 deletions.
diff --git a/cpp/src/gandiva/codegen/cache.h b/cpp/src/gandiva/codegen/cache.h
@@ -25,19 +25,15 @@ template <class KeyType, typename ValueType>
 class Cache {
  public:
   Cache(size_t capacity = CACHE_SIZE) : cache_(capacity) {}
-  ValueType GetCachedModule(KeyType cache_key) {
+  ValueType GetModule(KeyType cache_key) {
     boost::optional<ValueType> result;
-    result = cache_.get(cache_key);
-    if (result != boost::none) {
-      return result.value();
-    }
     mtx_.lock();
     result = cache_.get(cache_key);
     mtx_.unlock();
     return result != boost::none ? result.value() : nullptr;
   }
 
-  void CacheModule(KeyType cache_key, ValueType module) {
+  void PutModule(KeyType cache_key, ValueType module) {
     mtx_.lock();
     cache_.insert(cache_key, module);
     mtx_.unlock();

diff --git a/cpp/src/gandiva/codegen/filter.cc b/cpp/src/gandiva/codegen/filter.cc
@@ -46,7 +46,7 @@ Status Filter::Make(SchemaPtr schema, ConditionPtr condition,
                                   Status::Invalid("configuration cannot be null"));
   static Cache<FilterCacheKey, std::shared_ptr<Filter>> cache;
   FilterCacheKey cacheKey(schema, configuration, *(condition.get()));
-  std::shared_ptr<Filter> cachedFilter = cache.GetCachedModule(cacheKey);
+  std::shared_ptr<Filter> cachedFilter = cache.GetModule(cacheKey);
   if (cachedFilter != nullptr) {
     *filter = cachedFilter;
     return Status::OK();
@@ -67,7 +67,7 @@ Status Filter::Make(SchemaPtr schema, ConditionPtr condition,
 
   // Instantiate the filter with the completely built llvm generator
   *filter = std::make_shared<Filter>(std::move(llvm_gen), schema, configuration);
-  cache.CacheModule(cacheKey, *filter);
+  cache.PutModule(cacheKey, *filter);
   return Status::OK();
 }
 

diff --git a/cpp/src/gandiva/codegen/projector.cc b/cpp/src/gandiva/codegen/projector.cc
@@ -53,7 +53,7 @@ Status Projector::Make(SchemaPtr schema, const ExpressionVector &exprs,
   // see if equivalent projector was already built
   static Cache<ProjectorCacheKey, std::shared_ptr<Projector>> cache;
   ProjectorCacheKey cache_key(schema, configuration, exprs);
-  std::shared_ptr<Projector> cached_projector = cache.GetCachedModule(cache_key);
+  std::shared_ptr<Projector> cached_projector = cache.GetModule(cache_key);
   if (cached_projector != nullptr) {
     *projector = cached_projector;
     return Status::OK();
@@ -84,7 +84,7 @@ Status Projector::Make(SchemaPtr schema, const ExpressionVector &exprs,
   // Instantiate the projector with the completely built llvm generator
   *projector = std::shared_ptr<Projector>(
       new Projector(std::move(llvm_gen), schema, output_fields, configuration));
-  cache.CacheModule(cache_key, *projector);
+  cache.PutModule(cache_key, *projector);
   return Status::OK();
 }