[SYCL] Separate cache dir for tests and move cache config logic (#6286)

This commit makes the following changes:

Makes each persistent device code cache test use a unique cache directory to prevent possible overlap.
Move the logic and caching of SYCL_CACHE_PERSISTENT and SYCL_CACHE_DIR to their own specializations of SYCLConfig. This removes the need for PersistentDeviceCodeCache::reparseConfig as the SYCLConfig specializations offer their own reset functionality.

Author: steffenlarsen

sycl/source/detail/config.hpp

@@ -378,6 +378,95 @@ template <> class SYCLConfig<SYCL_QUEUE_THREAD_POOL_SIZE> {
   }
 };
 
+template <> class SYCLConfig<SYCL_CACHE_PERSISTENT> {
+  using BaseT = SYCLConfigBase<SYCL_CACHE_PERSISTENT>;
+
+public:
+  static constexpr bool Default = false; // default is disabled
+
+  static bool get() { return getCachedValue(); }
+
+  static void reset() { (void)getCachedValue(/*ResetCache=*/true); }
+
+  static const char *getName() { return BaseT::MConfigName; }
+
+private:
+  static bool parseValue() {
+    // Check if deprecated opt-out env var is used, then warn.
+    if (SYCLConfig<SYCL_CACHE_DISABLE_PERSISTENT>::get()) {
+      std::cerr
+          << "WARNING: " << SYCLConfig<SYCL_CACHE_DISABLE_PERSISTENT>::getName()
+          << " environment variable is deprecated "
+          << "and has no effect. By default, persistent device code caching is "
+          << (Default ? "enabled." : "disabled.") << " Use " << getName()
+          << "=1/0 to enable/disable.\n";
+    }
+
+    const char *ValStr = BaseT::getRawValue();
+    if (!ValStr)
+      return Default;
+    if (strlen(ValStr) != 1 || (ValStr[0] != '0' && ValStr[0] != '1')) {
+      std::string Msg =
+          std::string{"Invalid value for bool configuration variable "} +
+          getName() + std::string{": "} + ValStr;
+      throw runtime_error(Msg, PI_ERROR_INVALID_OPERATION);
+    }
+    return ValStr[0] == '1';
+  }
+
+  static bool getCachedValue(bool ResetCache = false) {
+    static bool Val = parseValue();
+    if (ResetCache)
+      Val = parseValue();
+    return Val;
+  }
+};
+
+template <> class SYCLConfig<SYCL_CACHE_DIR> {
+  using BaseT = SYCLConfigBase<SYCL_CACHE_DIR>;
+
+public:
+  static std::string get() { return getCachedValue(); }
+
+  static void reset() { (void)getCachedValue(/*ResetCache=*/true); }
+
+  static const char *getName() { return BaseT::MConfigName; }
+
+private:
+  // If environment variables are not available return an empty string to
+  // identify that cache is not available.
+  static std::string parseValue() {
+    const char *RootDir = BaseT::getRawValue();
+    if (RootDir)
+      return RootDir;
+
+    constexpr char DeviceCodeCacheDir[] = "/libsycl_cache";
+
+#if defined(__SYCL_RT_OS_LINUX)
+    const char *CacheDir = std::getenv("XDG_CACHE_HOME");
+    const char *HomeDir = std::getenv("HOME");
+    if (!CacheDir && !HomeDir)
+      return {};
+    std::string Res{
+        std::string(CacheDir ? CacheDir : (std::string(HomeDir) + "/.cache")) +
+        DeviceCodeCacheDir};
+#else
+    const char *AppDataDir = std::getenv("AppData");
+    if (!AppDataDir)
+      return {};
+    std::string Res{std::string(AppDataDir) + DeviceCodeCacheDir};
+#endif
+    return Res;
+  }
+
+  static std::string getCachedValue(bool ResetCache = false) {
+    static std::string Val = parseValue();
+    if (ResetCache)
+      Val = parseValue();
+    return Val;
+  }
+};
+
 } // namespace detail
 } // namespace sycl
 } // __SYCL_INLINE_NAMESPACE(cl)

sycl/source/detail/persistent_device_code_cache.cpp

@@ -65,6 +65,7 @@ bool PersistentDeviceCodeCache::isImageCached(const RTDeviceBinaryImage &Img) {
     return false;
   }
 
+  // TODO: Move parsing logic and caching to specializations of SYCLConfig.
   static auto MaxImgSize = getNumParam<SYCL_CACHE_MAX_DEVICE_IMAGE_SIZE>(
       DEFAULT_MAX_DEVICE_IMAGE_SIZE);
   static auto MinImgSize = getNumParam<SYCL_CACHE_MIN_DEVICE_IMAGE_SIZE>(
@@ -326,7 +327,7 @@ bool PersistentDeviceCodeCache::isCacheItemSrcEqual(
 std::string PersistentDeviceCodeCache::getCacheItemPath(
     const device &Device, const RTDeviceBinaryImage &Img,
     const SerializedObj &SpecConsts, const std::string &BuildOptionsString) {
-  static std::string cache_root{getRootDir()};
+  std::string cache_root{getRootDir()};
   if (cache_root.empty()) {
     trace("Disable persistent cache due to unconfigured cache root.");
     return {};
@@ -345,99 +346,22 @@ std::string PersistentDeviceCodeCache::getCacheItemPath(
          std::to_string(StringHasher(BuildOptionsString));
 }
 
-// TODO Currently parsing configuration variables and error reporting is not
-// centralized, and is basically re-implemented (with different level of
-// reliability) for each particular variable. As a variant, this can go into
-// the SYCLConfigBase class, which can be templated by value type, default value
-// and value parser (combined with error checker). It can also have typed get()
-// function returning one-time parsed and error-checked value.
-
-// Parses persistent cache configuration and checks it for errors.
-// Returns true if it is enabled, false otherwise.
-static bool parsePersistentCacheConfig() {
-  constexpr bool Default = false; // default is disabled
-
-  // Check if deprecated opt-out env var is used, then warn.
-  if (SYCLConfig<SYCL_CACHE_DISABLE_PERSISTENT>::get()) {
-    std::cerr
-        << "WARNING: " << SYCLConfig<SYCL_CACHE_DISABLE_PERSISTENT>::getName()
-        << " environment variable is deprecated "
-        << "and has no effect. By default, persistent device code caching is "
-        << (Default ? "enabled." : "disabled.") << " Use "
-        << SYCLConfig<SYCL_CACHE_PERSISTENT>::getName()
-        << "=1/0 to enable/disable.\n";
-  }
-  bool Ret = Default;
-  const char *RawVal = SYCLConfig<SYCL_CACHE_PERSISTENT>::get();
-
-  if (RawVal) {
-    if (!std::strcmp(RawVal, "0")) {
-      Ret = false;
-    } else if (!std::strcmp(RawVal, "1")) {
-      Ret = true;
-    } else {
-      std::string Msg =
-          std::string{"Invalid value for bool configuration variable "} +
-          SYCLConfig<SYCL_CACHE_PERSISTENT>::getName() + std::string{": "} +
-          RawVal;
-      throw runtime_error(Msg, PI_ERROR_INVALID_OPERATION);
-    }
-  }
-  PersistentDeviceCodeCache::trace(Ret ? "enabled" : "disabled");
-  return Ret;
-}
-
-/* Cached static variable signalling if the persistent cache is enabled.
- * The variable can have three values:
- *  - None  : The configuration has not been parsed.
- *  - true  : The persistent cache is enabled.
- *  - false : The persistent cache is disabled.
- */
-static std::optional<bool> CacheIsEnabled;
-
-/* Forces a reparsing of the information used to determine if the persistent
- * cache is enabled. This is primarily used for unit-testing where the
- * corresponding configuration variable is set by the individual tests.
- */
-void PersistentDeviceCodeCache::reparseConfig() {
-  CacheIsEnabled = parsePersistentCacheConfig();
-}
-
 /* Returns true if persistent cache is enabled.
  */
 bool PersistentDeviceCodeCache::isEnabled() {
-  if (!CacheIsEnabled)
-    reparseConfig();
-  return *CacheIsEnabled;
+  bool CacheIsEnabled = SYCLConfig<SYCL_CACHE_PERSISTENT>::get();
+  static bool FirstCheck = true;
+  if (FirstCheck) {
+    PersistentDeviceCodeCache::trace(CacheIsEnabled ? "enabled" : "disabled");
+    FirstCheck = false;
+  }
+  return CacheIsEnabled;
 }
 
 /* Returns path for device code cache root directory
- * If environment variables are not available return an empty string to identify
- * that cache is not available.
  */
 std::string PersistentDeviceCodeCache::getRootDir() {
-  static const char *RootDir = SYCLConfig<SYCL_CACHE_DIR>::get();
-  if (RootDir)
-    return RootDir;
-
-  constexpr char DeviceCodeCacheDir[] = "/libsycl_cache";
-
-  // Use static to calculate directory only once per program run
-#if defined(__SYCL_RT_OS_LINUX)
-  static const char *CacheDir = std::getenv("XDG_CACHE_HOME");
-  static const char *HomeDir = std::getenv("HOME");
-  if (!CacheDir && !HomeDir)
-    return {};
-  static std::string Res{
-      std::string(CacheDir ? CacheDir : (std::string(HomeDir) + "/.cache")) +
-      DeviceCodeCacheDir};
-#else
-  static const char *AppDataDir = std::getenv("AppData");
-  if (!AppDataDir)
-    return {};
-  static std::string Res{std::string(AppDataDir) + DeviceCodeCacheDir};
-#endif
-  return Res;
+  return SYCLConfig<SYCL_CACHE_DIR>::get();
 }
 
 } // namespace detail

sycl/source/detail/persistent_device_code_cache.hpp

@@ -184,12 +184,6 @@ class PersistentDeviceCodeCache {
                             const std::string &BuildOptionsString,
                             const RT::PiProgram &NativePrg);
 
-  /* Forces a reparsing of the information used to determine if the persistent
-   * cache is enabled. This is primarily used for unit-testing where the
-   * corresponding configuration variable is set by the individual tests.
-   */
-  static void reparseConfig();
-
   /* Sends message to std:cerr stream when SYCL_CACHE_TRACE environemnt is set*/
   static void trace(const std::string &msg) {
     static const char *TraceEnabled = SYCLConfig<SYCL_CACHE_TRACE>::get();

sycl/unittests/kernel-and-program/PersistentDeviceCodeCache.cpp

@@ -97,6 +97,8 @@ class PersistentDeviceCodeCache
   std::optional<std::string> SYCLCachePersistentBefore;
   bool SYCLCachePersistentChanged = false;
 
+  std::string RootSYCLCacheDir;
+
   // Caches the initial value of the SYCL_CACHE_PERSISTENT environment variable
   // before overwriting it with the new value.
   // Tear-down will reset the environment variable.
@@ -116,14 +118,38 @@ class PersistentDeviceCodeCache
     // persistent cache.
     set_env("SYCL_CACHE_PERSISTENT", NewValue);
     sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
-    detail::PersistentDeviceCodeCache::reparseConfig();
     SYCLCachePersistentChanged = true;
   }
 
+  void AppendToSYCLCacheDirEnv(const char *SubDir) {
+    std::string NewSYCLCacheDirPath{RootSYCLCacheDir};
+    if (NewSYCLCacheDirPath.back() != '\\' && NewSYCLCacheDirPath.back() != '/')
+      NewSYCLCacheDirPath += '/';
+    NewSYCLCacheDirPath += SubDir;
+    set_env("SYCL_CACHE_DIR", NewSYCLCacheDirPath.c_str());
+    sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_DIR>::reset();
+  }
+
+  void ResetSYCLCacheDirEnv() {
+    set_env("SYCL_CACHE_DIR", RootSYCLCacheDir.c_str());
+    sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_DIR>::reset();
+  }
+
   void SetUp() override {
-    EXPECT_NE(getenv("SYCL_CACHE_DIR"), nullptr)
-        << "Please set SYCL_CACHE_DIR environment variable pointing to cache "
-           "location.";
+    if (Plt.is_host() || Plt.get_backend() != backend::opencl)
+      GTEST_SKIP();
+
+    if (RootSYCLCacheDir == "")
+      FAIL() << "Please set SYCL_CACHE_DIR environment variable pointing to "
+                "cache location.";
+
+    // Append the test name to the cache dir to prevent conflicts with other
+    // tests running in parallel.
+    AppendToSYCLCacheDirEnv(
+        ::testing::UnitTest::GetInstance()->current_test_info()->name());
+
+    // Enable persistent cache
+    SetSYCLCachePersistentEnv("1");
   }
 
   void TearDown() override {
@@ -132,17 +158,25 @@ class PersistentDeviceCodeCache
       SetSYCLCachePersistentEnv(SYCLCachePersistentBefore
                                     ? SYCLCachePersistentBefore->c_str()
                                     : nullptr);
+    ResetSYCLCacheDirEnv();
   }
 
   PersistentDeviceCodeCache() : Plt{default_selector()} {
-
     if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
       std::clog << "This test is only supported on OpenCL devices\n";
       std::clog << "Current platform is "
                 << Plt.get_info<info::platform::name>();
       return;
     }
 
+    char *SYCLCacheDir = getenv("SYCL_CACHE_DIR");
+    if (!SYCLCacheDir) {
+      std::clog << "This test requires the SYCL_CACHE_DIR environment variable "
+                   "to be set.";
+      return;
+    }
+    RootSYCLCacheDir = SYCLCacheDir;
+
     Mock = std::make_unique<unittest::PiMock>(Plt);
     Dev = Plt.get_devices()[0];
     Mock->redefine<detail::PiApiKind::piProgramGetInfo>(
@@ -155,12 +189,6 @@ class PersistentDeviceCodeCache
    *              vector above.
    *  ThreadCount - number of parallel executors used for the test*/
   void ConcurentReadWriteCache(unsigned char ProgramID, size_t ThreadCount) {
-    if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
-      return;
-    }
-
-    SetSYCLCachePersistentEnv("1");
-
     std::string BuildOptions{"--concurrent-access=" +
                              std::to_string(ThreadCount)};
     DeviceCodeID = ProgramID;
@@ -223,12 +251,6 @@ class PersistentDeviceCodeCache
 /* Checks that key values with \0 symbols are processed correctly
  */
 TEST_P(PersistentDeviceCodeCache, KeysWithNullTermSymbol) {
-  if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
-    return;
-  }
-
-  SetSYCLCachePersistentEnv("1");
-
   std::string Key{'1', '\0', '3', '4', '\0'};
   std::vector<unsigned char> SpecConst(Key.begin(), Key.end());
   std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
@@ -281,12 +303,6 @@ TEST_P(PersistentDeviceCodeCache, ConcurentReadWriteCacheBigItem) {
  *  - binary file is corrupted.
  */
 TEST_P(PersistentDeviceCodeCache, CorruptedCacheFiles) {
-  if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
-    return;
-  }
-
-  SetSYCLCachePersistentEnv("1");
-
   std::string BuildOptions{"--corrupted-file"};
   std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
       Dev, Img, {}, BuildOptions);
@@ -351,12 +367,6 @@ TEST_P(PersistentDeviceCodeCache, CorruptedCacheFiles) {
  *  - cache miss happens on read operation.
  */
 TEST_P(PersistentDeviceCodeCache, LockFile) {
-  if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
-    return;
-  }
-
-  SetSYCLCachePersistentEnv("1");
-
   std::string BuildOptions{"--obsolete-lock"};
   std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
       Dev, Img, {}, BuildOptions);
@@ -407,12 +417,6 @@ TEST_P(PersistentDeviceCodeCache, LockFile) {
 /* Checks cache behavior when filesystem read/write operations fail
  */
 TEST_P(PersistentDeviceCodeCache, AccessDeniedForCacheDir) {
-  if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
-    return;
-  }
-
-  SetSYCLCachePersistentEnv("1");
-
   std::string BuildOptions{"--build-options"};
   std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
       Dev, Img, {}, BuildOptions);