refactor: remove PageCacheGuard usage from bag_converter (#75)

* refactor: remove PageCacheGuard usage from bag_converter

The periodic page cache dropping via PageCacheGuard added complexity
without clear benefit, as fadvise_sequential_access already hints
the kernel to manage page cache efficiently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: remove unused page_cache_drop_interval constant

No longer needed after removing PageCacheGuard usage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: remove PageCacheGuard class and fadvise_drop_page_cache

No longer used after removing page cache guard usage from bag_converter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: otenim

src/bag_converter/include/bag_converter.hpp

@@ -54,9 +54,6 @@ inline constexpr bool keep_original_topics = false;
 // Progress logging interval
 inline constexpr size_t progress_log_interval = 1000;
 
-// Page cache drop interval (in messages)
-inline constexpr size_t page_cache_drop_interval = 10000;
-
 // Minimum valid header.stamp epoch (2000-01-01T00:00:00Z = 946684800 seconds)
 inline constexpr int64_t header_stamp_min_epoch_sec = 946'684'800;
 

src/bag_converter/include/memory_management.hpp

@@ -10,7 +10,6 @@
 #define BAG_CONVERTER__MEMORY_MANAGEMENT_HPP
 
 #include <string>
-#include <vector>
 
 namespace memory_management
 {
@@ -26,49 +25,6 @@ namespace memory_management
  */
 void fadvise_sequential_access(const std::string & path);
 
-/**
- * @brief Drop page cache for a file using posix_fadvise(POSIX_FADV_DONTNEED).
- *
- * Opens the file with a separate file descriptor, advises the kernel that the
- * cached pages are no longer needed, then closes the descriptor. This works
- * regardless of whether another FD (e.g. from rosbag2) is still open on the
- * same file — the advisory applies to the underlying page cache.
- *
- * @param path Path to the file whose cached pages should be evicted
- */
-void fadvise_drop_page_cache(const std::string & path);
-
-/**
- * @brief RAII guard that frees page cache for tracked files on destruction.
- *
- * Tracks file paths via track() and calls fadvise_drop_page_cache() for each on
- * destruction. Ensures page cache is freed regardless of how the scope is
- * exited (normal return, early error return, or Ctrl+C graceful shutdown).
- */
-class PageCacheGuard
-{
-public:
-  PageCacheGuard() = default;
-  ~PageCacheGuard() { drop(); }
-
-  PageCacheGuard(const PageCacheGuard &) = delete;
-  PageCacheGuard & operator=(const PageCacheGuard &) = delete;
-  PageCacheGuard(PageCacheGuard &&) = delete;
-  PageCacheGuard & operator=(PageCacheGuard &&) = delete;
-
-  void track(const std::string & path) { paths_.push_back(path); }
-
-  void drop()
-  {
-    for (const auto & p : paths_) {
-      fadvise_drop_page_cache(p);
-    }
-  }
-
-private:
-  std::vector<std::string> paths_;
-};
-
 }  // namespace memory_management
 
 #endif  // BAG_CONVERTER__MEMORY_MANAGEMENT_HPP

src/bag_converter/src/bag_converter.cpp

@@ -596,10 +596,6 @@ BagConverterResultStatus run_impl(const BagConverterConfig & config)
     return BagConverterResultStatus::kError;
   }
 
-  // RAII guard: frees page cache for tracked files on any exit path
-  memory_management::PageCacheGuard cache_guard;
-  cache_guard.track(config.src_bag_path);
-
   const auto bag_metadata = reader.get_metadata();
 
   // Create TF transformer if frame is specified
@@ -666,8 +662,6 @@ BagConverterResultStatus run_impl(const BagConverterConfig & config)
     RCLCPP_ERROR(g_logger, "Error opening output bag: %s", e.what());
     return BagConverterResultStatus::kError;
   }
-  cache_guard.track(dst_path.string());
-
   // Create output topics based on input metadata
   std::set<std::string> created_topics;
   for (const auto & topic_info : bag_metadata.topics_with_message_count) {
@@ -768,9 +762,6 @@ BagConverterResultStatus run_impl(const BagConverterConfig & config)
       if (message_count % defaults::progress_log_interval == 0) {
         RCLCPP_INFO(g_logger, "Processed %zu messages...", message_count);
       }
-      if (message_count % defaults::page_cache_drop_interval == 0) {
-        cache_guard.drop();
-      }
       continue;
     }
 
@@ -789,9 +780,6 @@ BagConverterResultStatus run_impl(const BagConverterConfig & config)
     if (message_count % defaults::progress_log_interval == 0) {
       RCLCPP_INFO(g_logger, "Processed %zu messages...", message_count);
     }
-    if (message_count % defaults::page_cache_drop_interval == 0) {
-      cache_guard.drop();
-    }
   }
 
   if (!rclcpp::ok()) {
@@ -1024,13 +1012,6 @@ static int64_t merge_and_convert_group(
   const std::vector<fs::path> & bag_files, const fs::path & output_path,
   const std::string & storage_identifier, const BagConverterConfig & config)
 {
-  // RAII guard: frees page cache for all tracked files on any exit path
-  memory_management::PageCacheGuard cache_guard;
-  for (const auto & bag_path : bag_files) {
-    cache_guard.track(bag_path.string());
-  }
-  cache_guard.track(output_path.string());
-
   // 1. Collect topic union
   auto topic_union = merge::collect_topic_union(bag_files);
   if (!topic_union.has_value()) {
@@ -1221,10 +1202,6 @@ static int64_t merge_and_convert_group(
     if (message_count % defaults::progress_log_interval == 0) {
       RCLCPP_INFO(g_logger, "  Processed %ld messages...", message_count);
     }
-    if (message_count % defaults::page_cache_drop_interval == 0) {
-      cache_guard.drop();
-    }
-
     // Read next message from the same reader
     auto & reader = readers[entry.reader_index];
     if (reader->has_next()) {

src/bag_converter/src/memory_management.cpp

@@ -37,13 +37,4 @@ void fadvise_sequential_access(const std::string & path)
 #endif
 }
 
-void fadvise_drop_page_cache(const std::string & path)
-{
-#ifdef __linux__
-  fadvise(path, POSIX_FADV_DONTNEED);
-#else
-  (void)path;
-#endif
-}
-
 }  // namespace memory_management