Hoist entries to base class

This changeset moves the vector of entries from the specialized caches to the base cache class.
This will help refactoring and simplication and allow removing all the searchKernel methods in the subsequent changesets.
In particular this requires breaking out the dependent CachedEntry types and making them standalone types that can be passed as template parameters to the base Cache class.
Private visibility and friends are also removed.

Author: nicolasvasilache

tc/core/compilation_cache.h

@@ -56,7 +56,7 @@ struct TensorInfo {
 };
 } // namespace detail
 
-template <typename CC>
+template <typename CC, typename CachedEntryType>
 class Cache {
  public:
   static void enableCache();
@@ -68,6 +68,12 @@ class Cache {
   static std::shared_ptr<CC> getCache();
   static bool cacheEnabled();
 
+  typename std::vector<CachedEntryType>::const_iterator begin() const {
+    return entries_.begin();
+  }
+  typename std::vector<CachedEntryType>::const_iterator end() const {
+    return entries_.end();
+  }
   size_t size() const;
   void clear();
 
@@ -78,6 +84,8 @@ class Cache {
  protected:
   // XXX:this should be a std or boost shared_mutex
   mutable std::mutex mtx_;
+
+  std::vector<CachedEntryType> entries_;
 };
 
 class CacheEntrySameKeyDifferentValue : public std::invalid_argument {

tc/core/cuda/cuda_compilation_cache-inl.h

@@ -25,27 +25,28 @@
 
 namespace tc {
 
-template <typename CC>
-void Cache<CC>::enableCache() {
+template <typename CC, typename CachedEntryType>
+void Cache<CC, CachedEntryType>::enableCache() {
   CC::getGlobalSharedCache() = std::make_shared<CC>();
 }
 
-template <typename CC>
-void Cache<CC>::disableCache() {
+template <typename CC, typename CachedEntryType>
+void Cache<CC, CachedEntryType>::disableCache() {
   CC::getGlobalSharedCache() = nullptr;
 }
 
-template <typename CC>
-std::shared_ptr<CC> Cache<CC>::getCache() {
+template <typename CC, typename CachedEntryType>
+std::shared_ptr<CC> Cache<CC, CachedEntryType>::getCache() {
   if (not cacheEnabled()) {
     throw std::runtime_error(
         "EnableCache or LoadCacheFromProtobuf must be called before using the cache.");
   }
   return CC::getGlobalSharedCache();
 }
 
-template <typename CC>
-void Cache<CC>::dumpCacheToProtobuf(const std::string& filename) {
+template <typename CC, typename CachedEntryType>
+void Cache<CC, CachedEntryType>::dumpCacheToProtobuf(
+    const std::string& filename) {
   std::fstream serialized(
       filename, std::ios::binary | std::ios::trunc | std::ios::out);
   if (!serialized) {
@@ -56,8 +57,9 @@ void Cache<CC>::dumpCacheToProtobuf(const std::string& filename) {
   }
 }
 
-template <typename CC>
-void Cache<CC>::loadCacheFromProtobuf(const std::string& filename) {
+template <typename CC, typename CachedEntryType>
+void Cache<CC, CachedEntryType>::loadCacheFromProtobuf(
+    const std::string& filename) {
   typename CC::Protobuf buf;
   struct stat buffer = {0};
   if (stat(filename.c_str(), &buffer) == 0) {
@@ -67,28 +69,28 @@ void Cache<CC>::loadCacheFromProtobuf(const std::string& filename) {
   loadCacheFromProtobuf(buf);
 }
 
-template <typename CC>
+template <typename CC, typename CachedEntryType>
 template <typename Protobuf>
-void Cache<CC>::loadCacheFromProtobuf(const Protobuf& buf) {
+void Cache<CC, CachedEntryType>::loadCacheFromProtobuf(const Protobuf& buf) {
   static_assert(
       std::is_same<Protobuf, typename CC::Protobuf>::value,
       "LoadCacheFromProtobuf called with invalide protobuf type.");
   CC::getGlobalSharedCache() = std::make_shared<CC>(buf);
 }
 
-template <typename CC>
-bool Cache<CC>::cacheEnabled() {
+template <typename CC, typename CachedEntryType>
+bool Cache<CC, CachedEntryType>::cacheEnabled() {
   return CC::getGlobalSharedCache() != nullptr;
 }
 
-template <typename CC>
-size_t Cache<CC>::size() const {
+template <typename CC, typename CachedEntryType>
+size_t Cache<CC, CachedEntryType>::size() const {
   std::lock_guard<std::mutex> lock(mtx_);
   return static_cast<const CC*>(this)->entries_.size();
 }
 
-template <typename CC>
-void Cache<CC>::clear() {
+template <typename CC, typename CachedEntryType>
+void Cache<CC, CachedEntryType>::clear() {
   std::lock_guard<std::mutex> lock(mtx_);
   numberAttemptedRetrievals = numberSuccessfulRetrievals = numberCacheAttemps =
       0;
@@ -186,5 +188,4 @@ auto ManualCudaCache::searchKernelImpl(
   }
   return nullptr;
 }
-
 } // namespace tc

tc/core/cuda/cuda_compilation_cache.cc

@@ -78,7 +78,7 @@ CudaCache::CudaCache(const CudaCacheProto& buf) {
     entries_.emplace_back(entry_buf);
 }
 
-CudaCache::CachedEntry::CachedEntry(
+CudaCachedEntry::CudaCachedEntry(
     const std::string& id,
     const std::string& kernelSpecializedName,
     const std::vector<int>& kernelParameters,
@@ -98,7 +98,7 @@ CudaCache::CachedEntry::CachedEntry(
       values{cudaSource, kernelSpecializedName, kernelParameters, grid, block} {
 }
 
-CudaCache::CachedEntry::CachedEntry(const CudaCacheEntryProto& buf)
+CudaCachedEntry::CudaCachedEntry(const CudaCacheEntryProto& buf)
     : key{buf.id(),
           CudaMappingOptions{buf.kernel_options()},
           ProtoToTensorInfoVector(buf.inputs()),
@@ -146,23 +146,23 @@ void CudaCache::cacheKernel(
       CudaGPUInfo::GPUInfo().GetCudaDeviceStr());
 }
 
-CudaCache::CachedEntry* CudaCache::searchKernel(
+CudaCachedEntry* CudaCache::searchKernel(
     const std::string& id,
     const CudaMappingOptions& options,
     const std::vector<detail::TensorInfo>& inputs,
     const std::vector<detail::TensorInfo>& outputs) {
   return searchKernelImpl(*this, id, options, inputs, outputs);
 }
 
-CudaCache::CachedEntry* CudaCache::searchKernel(
+CudaCachedEntry* CudaCache::searchKernel(
     const std::string& id,
     const CudaMappingOptions& options,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs) {
   return searchKernelImpl(*this, id, options, inputs, outputs);
 }
 
-const CudaCache::CachedEntry* CudaCache::searchKernel(
+const CudaCachedEntry* CudaCache::searchKernel(
     const std::string& id,
     const CudaMappingOptions& options,
     const std::vector<const DLTensor*>& inputs,
@@ -351,21 +351,21 @@ void OptionsCache::recordRuntime(
   v->recordedRuntimes.push_back(runtime);
 }
 
-OptionsCache::CachedEntry* OptionsCache::searchKernel(
+OptionsCachedEntry* OptionsCache::searchKernel(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs) {
   return searchKernelImpl(*this, id, inputs, outputs);
 }
 
-const OptionsCache::CachedEntry* OptionsCache::searchKernel(
+const OptionsCachedEntry* OptionsCache::searchKernel(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs) const {
   return searchKernelImpl(*this, id, inputs, outputs);
 }
 
-OptionsCache::CachedEntry::CachedEntry(
+OptionsCachedEntry::OptionsCachedEntry(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs,
@@ -376,7 +376,7 @@ OptionsCache::CachedEntry::CachedEntry(
   values.emplace_back(options, runtime);
 }
 
-OptionsCache::CachedEntry::Key::Key(
+OptionsCachedEntry::Key::Key(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs_,
     const std::vector<const DLTensor*>& outputs_,
@@ -388,7 +388,7 @@ OptionsCache::CachedEntry::Key::Key(
           deviceStr,
           gitVersion) {}
 
-OptionsCache::CachedEntry::Key::Key(
+OptionsCachedEntry::Key::Key(
     const std::string& id,
     std::vector<detail::TensorInfo>&& inputs_,
     std::vector<detail::TensorInfo>&& outputs_,
@@ -400,12 +400,12 @@ OptionsCache::CachedEntry::Key::Key(
       deviceStr(deviceStr),
       gitVersion(gitVersion) {}
 
-OptionsCache::CachedEntry::Values::Values(
+OptionsCachedEntry::Values::Values(
     const CudaMappingOptions& options,
     Duration runtime)
     : mappingOptions(options), recordedRuntimes{runtime} {}
 
-OptionsCache::CachedEntry::Values::Values(
+OptionsCachedEntry::Values::Values(
     const CudaMappingOptions& options,
     std::vector<Duration>&& runtimes)
     : mappingOptions(options), recordedRuntimes(std::move(runtimes)) {}
@@ -416,29 +416,21 @@ OptionsCache::OptionsCache(const OptionsCacheProto& buf) {
     entries_.emplace_back(entry_buf);
 }
 
-decltype(OptionsCache::entries_)::const_iterator OptionsCache::begin() const {
-  return entries_.begin();
-}
-
-decltype(OptionsCache::entries_)::const_iterator OptionsCache::end() const {
-  return entries_.end();
-}
-
-OptionsCache::CachedEntry::CachedEntry(const OptionsCacheEntryProto& buf)
+OptionsCachedEntry::OptionsCachedEntry(const OptionsCacheEntryProto& buf)
     : key(buf.id(),
           ProtoToTensorInfoVector(buf.inputs()),
           ProtoToTensorInfoVector(buf.outputs()),
           buf.device_str(),
           buf.git_version()) {
   if (buf.values_size() == 0) {
     throw std::invalid_argument(
-        "OptionsCache::CachedEntry invalid protobuf: each entry should have at least one value field.");
+        "OptionsCachedEntry invalid protobuf: each entry should have at least one value field.");
   }
 
   for (const auto& value : buf.values()) {
     if (value.recorded_runtimes_size() == 0) {
       throw std::invalid_argument(
-          "OptionsCache::CachedEntry invalid protobuf: each entry value should have at least one recorded runtime.");
+          "OptionsCachedEntry invalid protobuf: each entry value should have at least one recorded runtime.");
     }
     std::vector<Duration> runtimes;
     runtimes.reserve(value.recorded_runtimes_size());
@@ -464,7 +456,7 @@ OptionsCacheProto OptionsCache::toProtobuf() const {
   return buf;
 }
 
-OptionsCacheEntryProto OptionsCache::CachedEntry::toProtobuf() const {
+OptionsCacheEntryProto OptionsCachedEntry::toProtobuf() const {
   OptionsCacheEntryProto buf;
   buf.set_id(key.id);
   std::transform(
@@ -509,7 +501,7 @@ CudaCacheProto CudaCache::toProtobuf() const {
   return buf;
 }
 
-CudaCacheEntryProto CudaCache::CachedEntry::toProtobuf() const {
+CudaCacheEntryProto CudaCachedEntry::toProtobuf() const {
   CudaCacheEntryProto buf;
   buf.set_id(key.id);
   *buf.mutable_kernel_options() = key.mappingOptions.proto();
@@ -560,14 +552,14 @@ std::unique_ptr<CudaCache::RetrievalResult> ManualCudaCache::retrieveKernel(
                                      entry->values.block});
 }
 
-ManualCudaCache::CachedEntry* ManualCudaCache::searchKernel(
+ManualCudaCachedEntry* ManualCudaCache::searchKernel(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs) {
   return searchKernelImpl(*this, id, inputs, outputs);
 }
 
-const ManualCudaCache::CachedEntry* ManualCudaCache::searchKernel(
+const ManualCudaCachedEntry* ManualCudaCache::searchKernel(
     const std::string& id,
     const std::vector<const DLTensor*>& inputs,
     const std::vector<const DLTensor*>& outputs) const {
@@ -606,7 +598,7 @@ void ManualCudaCache::cacheKernel(
       cudaSource,
       CudaGPUInfo::GPUInfo().GetCudaDeviceStr());
 }
-ManualCudaCache::CachedEntry::CachedEntry(
+ManualCudaCachedEntry::ManualCudaCachedEntry(
     const std::string& id,
     const std::string& kernelSpecializedName,
     const std::vector<int>& kernelParameters,