Some more PR cleanup

Author: ypatia

test/src/unit-sparse-unordered-with-dups-reader.cc

@@ -1065,11 +1065,13 @@ TEST_CASE_METHOD(
     if (one_frag) {
       CHECK(1 == loop_num->second);
     }
-
-    // FIXME: This check has become unpredictable, see why the loop number is
-    // not consistent } else {
-    //   CHECK(20 == loop_num->second);
-    // }
+    /**
+     * FIXME: The loop_num appears to be different on different architectures/build modes.
+     * SC-61065 to investigate why.
+     * } else {
+     *   CHECK(20 == loop_num->second);
+     * }
+     */
 
     // Try to read multiple frags without partial tile offset reading. Should
     // fail

tiledb/sm/query/readers/filtered_data.h

@@ -415,7 +415,7 @@ class FilteredData {
           auto timer_se = stats_->start_timer("read");
           return resources_.vfs().read(uri, offset, data, size, false);
         });
-    // This should be changes once we use taskgraphs for modeling the data flow
+    // This should be changed once we use taskgraphs for modeling the data flow
     block.set_io_task(task);
   }
 

tiledb/sm/query/readers/reader_base.cc

@@ -969,6 +969,7 @@ Status ReaderBase::unfilter_tiles(
             return Status::Ok();
           }
 
+          // The current threadpool design does not allow for unfiltering to happen in chunks using a parallel for within this async task as the wait_all in the end of the parallel for can deadlock.
           for (uint64_t range_thread_idx = 0;
                range_thread_idx < num_range_threads;
                range_thread_idx++) {
@@ -998,6 +999,7 @@ Status ReaderBase::unfilter_tiles(
       continue;
     }
 
+    // Unfiltering tasks have been launched, set the tasks to wait for in the corresponding tiles. When those tasks(futures) will be ready the tile processing that depends on the unfiltered tile will get unblocked. 
     auto tile_tuple = result_tile->tile_tuple(name);
     tile_tuple->fixed_tile().set_unfilter_data_compute_task(task);
 

tiledb/sm/query/readers/result_tile.h

@@ -228,17 +228,20 @@ class ResultTile {
     }
 
     ~TileData() {
-      // TODO: destructor should not throw, catch any exceptions
-      if (fixed_filtered_data_task_.valid()) {
-        auto st = fixed_filtered_data_task_.wait();
-      }
-
-      if (var_filtered_data_task_.valid()) {
-        auto st = var_filtered_data_task_.wait();
-      }
-
-      if (validity_filtered_data_task_.valid()) {
-        auto st = validity_filtered_data_task_.wait();
+      try {
+        if (fixed_filtered_data_task_.valid()) {
+          auto st = fixed_filtered_data_task_.wait();
+        }
+
+        if (var_filtered_data_task_.valid()) {
+          auto st = var_filtered_data_task_.wait();
+        }
+
+        if (validity_filtered_data_task_.valid()) {
+          auto st = validity_filtered_data_task_.wait();
+        }
+      } catch (...) {
+        return;
       }
     }
 

tiledb/sm/tile/tile.cc

@@ -32,7 +32,6 @@
 
 #include "tiledb/sm/tile/tile.h"
 
-#include <utility>
 #include "tiledb/common/exception/exception.h"
 #include "tiledb/common/heap_memory.h"
 #include "tiledb/common/memory_tracker.h"

tiledb/sm/tile/tile.h

@@ -107,9 +107,11 @@ class TileBase {
     return static_cast<T*>(data());
   }
 
-  /** Converts the data pointer to a specific type with no check on compute
+  /**
+   * Converts the data pointer to a specific type with no check on compute
    * task. This is used for getting thte data from inside the compute thread
-   * itself for unfiltering. */
+   * itself for unfiltering. 
+   */
   template <class T>
   inline T* data_as_unsafe() const {
     return static_cast<T*>(data_unsafe());
@@ -134,8 +136,10 @@ class TileBase {
     return data_.get();
   }
 
-  /** Returns the internal buffer. This is used for getting thte data from
-   * inside the compute thread itself for unfiltering. */
+  /**
+   * Returns the internal buffer. This is used for getting thte data from
+   * inside the compute thread itself for unfiltering.
+   */
   inline void* data_unsafe() const {
     return data_.get();
   }
@@ -198,7 +202,8 @@ class TileBase {
   /** The tile data type. */
   Datatype type_;
 
-  /** Whether to block waiting for io data to be ready before accessing data()
+  /**
+   * Whether to block waiting for io data to be ready before accessing data()
    */
   const bool skip_waiting_on_io_task_;