refactor(cache): move rwlock from LruEntry to FileStore, to reduce memory usage (#1149) (#1158)

photonlibos · xiaoyang-hhh · web-flow · commit 2b1e1ffe3d37 · 2026-03-20T11:10:50.000+08:00
* refactor(cache): lazy initialize rwlock to reduce memory usage

* fix build

* use defer to release

Co-authored-by: xiaoyang-hhh &lt;luxiaoyang.lxy@alibaba-inc.com&gt;
diff --git a/fs/cache/full_file_cache/cache_pool.cpp b/fs/cache/full_file_cache/cache_pool.cpp
@@ -135,8 +135,10 @@ int FileCachePool::evict(std::string_view filename) {
     lru_.mark_key_cleared(lruEntry->lruIter);
   }
   int err = 0;
+  auto cacheStore = static_cast<FileCacheStore*>(open(filePath, O_RDWR, 0644));
+  DEFER(cacheStore->release());
   {
-    photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);
+    photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);
     err = mediaFs_->truncate(filePath.data(), 0);
     lruEntry->truncate_done = false;
   }
@@ -256,8 +258,10 @@ void FileCachePool::eviction() {
         continue;
     }
 
+    auto cacheStore = static_cast<FileCacheStore*>(open(fileName, O_RDWR, 0644));
+    DEFER(cacheStore->release());
     {
-      photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);
+      photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);
       err = mediaFs_->truncate(fileName.data(), 0);
       lruEntry->truncate_done = false;
     }
diff --git a/fs/cache/full_file_cache/cache_pool.h b/fs/cache/full_file_cache/cache_pool.h
@@ -45,9 +45,6 @@ class FileCachePool : public photon::fs::ICachePool {
 
     void Init();
 
-    //  pathname must begin with '/'
-    photon::fs::ICacheStore *do_open(std::string_view pathname, int flags, mode_t mode) override;
-
     int set_quota(std::string_view pathname, size_t quota) override;
     int stat(photon::fs::CacheStat *stat,
              std::string_view pathname = std::string_view(nullptr, 0)) override;
@@ -64,7 +61,6 @@ class FileCachePool : public photon::fs::ICachePool {
         uint32_t lruIter;
         int openCount;
         uint64_t size;
-        photon::rwlock rw_lock_;
         bool truncate_done;
     };
 
@@ -81,6 +77,8 @@ class FileCachePool : public photon::fs::ICachePool {
     uint64_t updateSpace(FileNameMap::iterator iter, uint64_t size);
 
 protected:
+    //  pathname must begin with '/'
+    photon::fs::ICacheStore *do_open(std::string_view pathname, int flags, mode_t mode) override;
     photon::fs::IFile *openMedia(std::string_view name, int flags, int mode);
 
     static uint64_t timerHandler(void *data);
diff --git a/fs/cache/full_file_cache/cache_store.cpp b/fs/cache/full_file_cache/cache_store.cpp
@@ -51,8 +51,7 @@ FileCacheStore::~FileCacheStore() {
 
 ICacheStore::try_preadv_result FileCacheStore::try_preadv2(const struct iovec *iov, int iovcnt,
                                                            off_t offset, int flags) {
-  auto lruEntry = static_cast<FileCachePool::LruEntry *>(iterator_->second.get());
-  photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::RLOCK);
+  photon::scoped_rwlock rl(rw_lock_, photon::RLOCK);
   return this->ICacheStore::try_preadv2(iov, iovcnt, offset, flags);
 }
 
@@ -70,8 +69,8 @@ ssize_t FileCacheStore::do_pwritev(const struct iovec *iov, int iovcnt, off_t of
 {
   ssize_t ret;
   iovector_view view((iovec*)iov, iovcnt);
-  auto lruEntry = static_cast<FileCachePool::LruEntry *>(iterator_->second.get());
-  photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::RLOCK);
+  auto lruEntry = iterator_->second.get();
+  photon::scoped_rwlock rl(rw_lock_, photon::RLOCK);
   if (!lruEntry->truncate_done) {
     // May repeated ftruncate() here, but it doesn't matter
     ret = localFile_->ftruncate(actual_size_);
diff --git a/fs/cache/full_file_cache/cache_store.h b/fs/cache/full_file_cache/cache_store.h
@@ -47,6 +47,8 @@ class FileCacheStore : public photon::fs::ICacheStore {
 
     int fstat(struct stat *buf) override;
 
+    photon::rwlock &rw_lock() { return rw_lock_; }
+
 protected:
     bool cacheIsFull();
 
@@ -70,6 +72,8 @@ class FileCacheStore : public photon::fs::ICacheStore {
     FileIterator iterator_;
     RangeLock rangeLock_;
 
+    photon::rwlock rw_lock_;
+
     ssize_t do_pwritev(const struct iovec *iov, int iovcnt, off_t offset);
 };
 
diff --git a/fs/cache/full_file_cache/quota_pool.cpp b/fs/cache/full_file_cache/quota_pool.cpp
@@ -194,8 +194,11 @@ int QuotaFilePool::evict(std::string_view filename) {
   const auto& filePath = fileIter->first;
   int err;
   auto lruEntry = static_cast<QuotaLruEntry*>(fileIter->second.get());
+
+  auto cacheStore = static_cast<FileCacheStore*>(open(filePath, O_RDWR, 0644));
+  DEFER(cacheStore->release());
   {
-    photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);
+    photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);
     lru.mark_key_cleared(lruEntry->QuotaLruIter);
     err = mediaFs_->truncate(filePath.data(), 0);
     if (err) {
@@ -234,15 +237,19 @@ void QuotaFilePool::dirEviction() {
       auto lruEntry = static_cast<QuotaLruEntry*>(fileIter->second.get());
       int err;
       bool flags_dir_delete = false;
+
+      auto cacheStore = static_cast<FileCacheStore*>(open(fileName, O_RDWR, 0644));
+      DEFER(cacheStore->release());
       {
-        photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);
+        photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);
         if (lruEntry->openCount==0){
           dir->lru.mark_key_cleared(lruEntry->QuotaLruIter);
         } else {
           dir->lru.access(lruEntry->QuotaLruIter);
         }
         err = mediaFs_->truncate(fileName.data(), 0);
       }
+
       if (err) {
         ERRNO e;
         LOG_ERROR("truncate(0) failed, name : `, ret : `, error code : `", fileName, err, e);
diff --git a/fs/cache/full_file_cache/quota_pool.h b/fs/cache/full_file_cache/quota_pool.h
@@ -27,8 +27,6 @@ class QuotaFilePool : public FileCachePool {
     QuotaFilePool(photon::fs::IFileSystem* mediaFs, uint64_t capacityInGB, uint64_t periodInUs,
       uint64_t diskAvailInBytes, uint64_t refillUnit, int quotaDirLevel);
 
-    photon::fs::ICacheStore* do_open(std::string_view pathname, int flags, mode_t mode) override;
-
     void updateDirLru(FileIterator iter);
 
     bool dirSpaceIsFull(FileIterator iter);
@@ -42,6 +40,7 @@ class QuotaFilePool : public FileCachePool {
     int evict(std::string_view filename) override;
 
 protected:
+    photon::fs::ICacheStore* do_open(std::string_view pathname, int flags, mode_t mode) override;
     void dirEviction();
 
     bool afterFtrucate(FileIterator iter) override;
diff --git a/fs/cache/full_file_cache/quota_store.cpp b/fs/cache/full_file_cache/quota_store.cpp
@@ -56,8 +56,7 @@ ssize_t QuotaFileStore::do_pwritev2(const struct iovec *iov, int iovcnt, off_t o
   }
   ssize_t ret;
   {
-    auto lruEntry = static_cast<FileCachePool::LruEntry*>(iterator_->second.get());
-    photon::scoped_rwlock wl(lruEntry->rw_lock_, photon::WLOCK);
+    photon::scoped_rwlock wl(rw_lock_, photon::WLOCK);
     ret = localFile_->pwritev(iov, iovcnt, offset);
   }
   if (ret < 0 && ENOSPC == errno) {
diff --git a/fs/cache/pool_store.h b/fs/cache/pool_store.h
@@ -108,8 +108,6 @@ namespace fs
 
         void set_trans_func(CacheFnTransFunc fn_trans_func);
 
-        virtual ICacheStore* do_open(std::string_view filename, int flags, mode_t mode) = 0;
-
         virtual int rename(std::string_view oldname, std::string_view newname) = 0;
 
         virtual ssize_t list(const char* dirname, ListType type,
@@ -146,6 +144,8 @@ namespace fs
         }
 
     protected:
+        virtual ICacheStore* do_open(std::string_view filename, int flags, mode_t mode) = 0;
+
         void* m_stores;
         CacheFnTransFunc fn_trans_func;
         void* m_thread_pool = nullptr;
diff --git a/fs/cache/test/cache_test.cpp b/fs/cache/test/cache_test.cpp
@@ -676,6 +676,44 @@ TEST(CachePool, random_evict_file) {
   if (cacheStore) cacheStore->release();
 }
 
+TEST(CachePool, open_same_file) {
+  std::string root = "/tmp/ease/cache/open_same_file/";
+  SetupTestDir(root);
+  auto mediaFs = new_localfs_adaptor(root.c_str(), ioengine_libaio);
+  auto alignFs = new_aligned_fs_adaptor(mediaFs, 4 * 1024, true, true);
+  auto cacheAllocator = new AlignedAlloc(4 * 1024);
+  auto roCachedFs = new_full_file_cached_fs(nullptr, alignFs, 1024 * 1024,
+      1, 1000 * 1000 * 1, 128ul * 1024 * 1024, cacheAllocator, 0);
+  auto cachePool = roCachedFs->get_pool();
+  DEFER({ delete cacheAllocator; delete roCachedFs; });
+
+  auto fileName = "/testDir/testfile";
+
+  uint64_t written = 0;
+  for (int j = 0; j < 5; j++) {
+    std::vector<ICacheStore*> cacheStores;
+    for (int i = 0; i < 10; i++) {
+      cacheStores.push_back(cachePool->open(fileName, O_CREAT | O_RDWR, 0644));
+      ASSERT_NE(nullptr, cacheStores.back());
+    }
+
+    const size_t bufSize = 1024 * 1024;
+    IOVector buffer(*cacheAllocator);
+    buffer.push_back(bufSize);
+
+    for (auto cacheStore : cacheStores) {
+      if (rand() % 2 == 0) {
+        cacheStore->release();
+      } else {
+        auto ret = cacheStore->do_pwritev2(buffer.iovec(), buffer.iovcnt(), written, 0);
+        EXPECT_EQ(ret, (ssize_t)bufSize);
+        written += ret;
+        cacheStore->release();
+      }
+    }
+  }
+}
+
 }
 }
 int main(int argc, char** argv) {

Original file line number	Diff line number	Diff line change
`@@ -135,8 +135,10 @@ int FileCachePool::evict(std::string_view filename) {`
`135`	`135`	`lru_.mark_key_cleared(lruEntry->lruIter);`
`136`	`136`	`}`
`137`	`137`	`int err = 0;`
	`138`	`+ auto cacheStore = static_cast<FileCacheStore*>(open(filePath, O_RDWR, 0644));`
	`139`	`+ DEFER(cacheStore->release());`
`138`	`140`	`{`
`139`		`- photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);`
	`141`	`+ photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);`
`140`	`142`	`err = mediaFs_->truncate(filePath.data(), 0);`
`141`	`143`	`lruEntry->truncate_done = false;`
`142`	`144`	`}`
`@@ -256,8 +258,10 @@ void FileCachePool::eviction() {`
`256`	`258`	`continue;`
`257`	`259`	`}`
`258`	`260`
	`261`	`+ auto cacheStore = static_cast<FileCacheStore*>(open(fileName, O_RDWR, 0644));`
	`262`	`+ DEFER(cacheStore->release());`
`259`	`263`	`{`
`260`		`- photon::scoped_rwlock rl(lruEntry->rw_lock_, photon::WLOCK);`
	`264`	`+ photon::scoped_rwlock rl(cacheStore->rw_lock(), photon::WLOCK);`
`261`	`265`	`err = mediaFs_->truncate(fileName.data(), 0);`
`262`	`266`	`lruEntry->truncate_done = false;`
`263`	`267`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,8 +56,7 @@ ssize_t QuotaFileStore::do_pwritev2(const struct iovec *iov, int iovcnt, off_t o`
`56`	`56`	`}`
`57`	`57`	`ssize_t ret;`
`58`	`58`	`{`
`59`		`- auto lruEntry = static_cast<FileCachePool::LruEntry*>(iterator_->second.get());`
`60`		`- photon::scoped_rwlock wl(lruEntry->rw_lock_, photon::WLOCK);`
	`59`	`+ photon::scoped_rwlock wl(rw_lock_, photon::WLOCK);`
`61`	`60`	`ret = localFile_->pwritev(iov, iovcnt, offset);`
`62`	`61`	`}`
`63`	`62`	`if (ret < 0 && ENOSPC == errno) {`