Skip to content

Commit

Permalink
Cleaup PR
Browse files Browse the repository at this point in the history
  • Loading branch information
ypatia committed Dec 18, 2024
1 parent 1d379b1 commit 4723950
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 32 deletions.
8 changes: 4 additions & 4 deletions tiledb/sm/tile/tile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ TileBase::TileBase(
const uint64_t cell_size,
const uint64_t size,
tdb::pmr::memory_resource* resource,
const bool ignore_tasks)
const bool skip_waiting_on_io_task)
: resource_(resource)
, data_(tdb::pmr::make_unique<std::byte>(resource_, size))
, size_(size)
, cell_size_(cell_size)
, format_version_(format_version)
, type_(type)
, ignore_tasks_(ignore_tasks) {
, skip_waiting_on_io_task_(skip_waiting_on_io_task) {
/*
* We can check for a bad allocation after initialization without risk
* because none of the other member variables use its value for their own
Expand Down Expand Up @@ -211,7 +211,7 @@ WriterTile::WriterTile(

void TileBase::read(
void* const buffer, const uint64_t offset, const uint64_t nbytes) const {
if (!ignore_tasks_) {
if (!skip_waiting_on_io_task_) {
if (unfilter_data_compute_task_.valid()) {
throw_if_not_ok(unfilter_data_compute_task_.wait());
} else {
Expand Down Expand Up @@ -312,7 +312,7 @@ void WriterTile::write_var(const void* data, uint64_t offset, uint64_t nbytes) {

uint64_t Tile::load_chunk_data(
ChunkData& unfiltered_tile, uint64_t expected_original_size) {
// std::scoped_lock<std::recursive_mutex> lock{filtered_data_io_task_mtx_};
std::scoped_lock<std::recursive_mutex> lock{filtered_data_io_task_mtx_};
assert(filtered());

Deserializer deserializer(filtered_data(), filtered_size());
Expand Down
36 changes: 8 additions & 28 deletions tiledb/sm/tile/tile.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,26 +62,21 @@ class TileBase {
* @param cell_size The cell size.
* @param size The size of the tile.
* @param resource The memory resource to use.
* @param data_io_task The I/O task to wait on for data to be valid.
* @param skip_waiting_on_io_task whether to skip waiting on I/O tasks and
* directly access data() or block. By default is false, so by default we
* block waiting. Used when we create generic tiles or in testing.
*/
TileBase(
const format_version_t format_version,
const Datatype type,
const uint64_t cell_size,
const uint64_t size,
tdb::pmr::memory_resource* resource,
const bool ignore_tasks = false);
const bool skip_waiting_on_io_task = false);

DISABLE_COPY_AND_COPY_ASSIGN(TileBase);
DISABLE_MOVE_AND_MOVE_ASSIGN(TileBase);

~TileBase() {
// TODO: destructor should not throw, catch any exceptions
if (unfilter_data_compute_task_.valid()) {
auto st = unfilter_data_compute_task_.wait();
}
}

/* ********************************* */
/* API */
/* ********************************* */
Expand Down Expand Up @@ -118,7 +113,7 @@ class TileBase {

/** Returns the internal buffer. */
inline void* data() const {
if (!ignore_tasks_) {
if (!skip_waiting_on_io_task_) {
if (unfilter_data_compute_task_.valid()) {
throw_if_not_ok(unfilter_data_compute_task_.wait());
} else {
Expand Down Expand Up @@ -193,18 +188,12 @@ class TileBase {
/** The tile data type. */
Datatype type_;

const bool ignore_tasks_;
/** Whether to block waiting for io data to be ready before accessing data()
*/
const bool skip_waiting_on_io_task_;

/** Compute task to check and block on if unfiltered data is ready. */
mutable ThreadPool::SharedTask unfilter_data_compute_task_;

/**
* Lock for checking task, since this tile can be used by multiple threads.
* The ThreadPool::SharedTask lets multiple threads copy the task, but it
* doesn't let multiple threads access a single task itself. Due to this we
* need a mutext since the tile will be accessed by multiple threads.
*/
mutable std::recursive_mutex unfilter_data_compute_task_mtx_;
};

/**
Expand Down Expand Up @@ -284,13 +273,6 @@ class Tile : public TileBase {
DISABLE_MOVE_AND_MOVE_ASSIGN(Tile);
DISABLE_COPY_AND_COPY_ASSIGN(Tile);

~Tile() {
// TODO: destructor should not throw, catch any exceptions
if (unfilter_data_compute_task_.valid()) {
auto st = unfilter_data_compute_task_.wait();
}
}

/* ********************************* */
/* API */
/* ********************************* */
Expand Down Expand Up @@ -519,7 +501,6 @@ class WriterTile : public TileBase {
* @param cell_size The cell size.
* @param size The size of the tile.
* @param memory_tracker The memory tracker to use.
* @param data_io_task The I/O task to wait on for data to be valid.
*/
WriterTile(
const format_version_t format_version,
Expand All @@ -536,7 +517,6 @@ class WriterTile : public TileBase {
* @param cell_size The cell size.
* @param size The size of the tile.
* @param resource The memory resource to use.
* @param data_io_task The I/O task to wait on for data to be valid.
*/
WriterTile(
const format_version_t format_version,
Expand Down

0 comments on commit 4723950

Please sign in to comment.