Skip to content

Commit

Permalink
Remove move from Tile.
Browse files Browse the repository at this point in the history
This will help the memory tracking effort by removing all move constructors/assign operator for Tile, ResultTile, etc.

---
TYPE: NO_HISTORY
DESC: Remove move from Tile.
  • Loading branch information
KiterLuc committed Feb 21, 2024
1 parent a84789e commit 05cf973
Show file tree
Hide file tree
Showing 21 changed files with 146 additions and 353 deletions.
21 changes: 5 additions & 16 deletions test/src/unit-Reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,38 +276,27 @@ TEST_CASE_METHOD(
result_space_tiles);
CHECK(result_space_tiles.size() == 6);

// Result tiles for fragment #1
ResultTile result_tile_1_0_1(1, 0, *fragments[0]);
ResultTile result_tile_1_2_1(1, 2, *fragments[0]);

// Result tiles for fragment #2
ResultTile result_tile_1_0_2(2, 0, *fragments[1]);

// Result tiles for fragment #3
ResultTile result_tile_2_0_3(3, 0, *fragments[2]);
ResultTile result_tile_3_0_3(3, 2, *fragments[2]);

// Initialize result_space_tiles
ResultSpaceTile<int32_t> rst_1_0;
rst_1_0.set_start_coords({3, 1});
rst_1_0.append_frag_domain(2, ds2);
rst_1_0.append_frag_domain(1, ds1);
rst_1_0.set_result_tile(1, result_tile_1_0_1);
rst_1_0.set_result_tile(2, result_tile_1_0_2);
rst_1_0.set_result_tile(1, 0, *fragments[0]);
rst_1_0.set_result_tile(2, 0, *fragments[1]);
ResultSpaceTile<int32_t> rst_1_2;
rst_1_2.set_start_coords({3, 11});
rst_1_2.append_frag_domain(1, ds1);
rst_1_2.set_result_tile(1, result_tile_1_2_1);
rst_1_2.set_result_tile(1, 2, *fragments[0]);
ResultSpaceTile<int32_t> rst_2_0;
rst_2_0.set_start_coords({5, 1});
rst_2_0.append_frag_domain(3, ds3);
rst_2_0.set_result_tile(3, result_tile_2_0_3);
rst_2_0.set_result_tile(3, 0, *fragments[2]);
ResultSpaceTile<int32_t> rst_2_2;
rst_2_2.set_start_coords({5, 11});
ResultSpaceTile<int32_t> rst_3_0;
rst_3_0.set_start_coords({7, 1});
rst_3_0.append_frag_domain(3, ds3);
rst_3_0.set_result_tile(3, result_tile_3_0_3);
rst_3_0.set_result_tile(3, 2, *fragments[2]);
ResultSpaceTile<int32_t> rst_3_2;
rst_3_2.set_start_coords({7, 11});

Expand Down
15 changes: 7 additions & 8 deletions test/src/unit-filter-pipeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,14 @@ WriterTile make_offsets_tile(std::vector<uint64_t>& offsets) {
}

Tile create_tile_for_unfiltering(uint64_t nelts, WriterTile& tile) {
Tile ret(
return {
tile.format_version(),
tile.type(),
tile.cell_size(),
0,
tile.cell_size() * nelts,
tile.filtered_buffer().data(),
tile.filtered_buffer().size());
return ret;
tile.filtered_buffer().size()};
}

void run_reverse(
Expand Down Expand Up @@ -1221,19 +1220,19 @@ TEST_CASE("Filter: Test encryption", "[filter][encryption]") {
key[0]++;
filter->set_key(key);

unfiltered_tile = create_tile_for_unfiltering(nelts, tile);
run_reverse(config, tp, unfiltered_tile, pipeline, false);
auto unfiltered_tile2 = create_tile_for_unfiltering(nelts, tile);
run_reverse(config, tp, unfiltered_tile2, pipeline, false);

// Fix key and check success.
unfiltered_tile = create_tile_for_unfiltering(nelts, tile);
auto unfiltered_tile3 = create_tile_for_unfiltering(nelts, tile);
key[0]--;
filter->set_key(key);
run_reverse(config, tp, unfiltered_tile, pipeline);
run_reverse(config, tp, unfiltered_tile3, pipeline);

for (uint64_t i = 0; i < nelts; i++) {
uint64_t elt = 0;
CHECK_NOTHROW(
unfiltered_tile.read(&elt, i * sizeof(uint64_t), sizeof(uint64_t)));
unfiltered_tile3.read(&elt, i * sizeof(uint64_t), sizeof(uint64_t)));
CHECK(elt == i);
}
}
Expand Down
14 changes: 8 additions & 6 deletions test/src/unit-sparse-unordered-with-dups-reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1527,7 +1527,7 @@ TEST_CASE_METHOD(
auto&& [array, fragments] = open_default_array_1d_with_fragments(capacity);

// Make a vector of tiles.
std::vector<UnorderedWithDupsResultTile<uint64_t>> rt;
std::list<UnorderedWithDupsResultTile<uint64_t>> rt;
for (uint64_t t = 0; t < num_tiles; t++) {
rt.emplace_back(0, t, *fragments[0]);

Expand All @@ -1540,8 +1540,9 @@ TEST_CASE_METHOD(

// Create the result_tiles pointer vector.
std::vector<ResultTile*> result_tiles(rt.size());
for (uint64_t i = 0; i < rt.size(); i++) {
result_tiles[i] = &rt[i];
uint64_t i = 0;
for (auto& t : rt) {
result_tiles[i++] = &t;
}

// Create a Query buffer.
Expand Down Expand Up @@ -1743,7 +1744,7 @@ TEST_CASE_METHOD(
auto&& [array, fragments] = open_default_array_1d_with_fragments(capacity);

// Make a vector of tiles.
std::vector<UnorderedWithDupsResultTile<uint64_t>> rt;
std::list<UnorderedWithDupsResultTile<uint64_t>> rt;
for (uint64_t t = 0; t < num_tiles; t++) {
rt.emplace_back(0, t, *fragments[0]);

Expand All @@ -1756,8 +1757,9 @@ TEST_CASE_METHOD(

// Create the result_tiles pointer vector.
std::vector<ResultTile*> result_tiles(rt.size());
for (uint64_t i = 0; i < rt.size(); i++) {
result_tiles[i] = &rt[i];
uint64_t i = 0;
for (auto& t : rt) {
result_tiles[i++] = &t;
}

// Call the function.
Expand Down
4 changes: 1 addition & 3 deletions tiledb/sm/array/array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1478,10 +1478,8 @@ void Array::do_load_metadata() {
throw_if_not_ok(
parallel_for(&resources_.compute_tp(), 0, metadata_num, [&](size_t m) {
const auto& uri = array_metadata_to_load[m].uri_;

auto&& tile =
metadata_tiles[m] =
GenericTileIO::load(resources_, uri, 0, *encryption_key());
metadata_tiles[m] = tdb::make_shared<Tile>(HERE(), std::move(tile));

return Status::Ok();
}));
Expand Down
16 changes: 8 additions & 8 deletions tiledb/sm/array/array_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ shared_ptr<ArraySchema> ArrayDirectory::load_array_schema_from_uri(
auto timer_se =
resources.stats().start_timer("sm_load_array_schema_from_uri");

auto&& tile = GenericTileIO::load(resources, schema_uri, 0, encryption_key);
auto tile = GenericTileIO::load(resources, schema_uri, 0, encryption_key);

resources.stats().add_counter("read_array_schema_size", tile.size());
resources.stats().add_counter("read_array_schema_size", tile->size());

// Deserialize
Deserializer deserializer(tile.data(), tile.size());
Deserializer deserializer(tile->data(), tile->size());
return make_shared<ArraySchema>(
HERE(), ArraySchema::deserialize(deserializer, schema_uri));
}
Expand Down Expand Up @@ -1322,18 +1322,18 @@ shared_ptr<const Enumeration> ArrayDirectory::load_enumeration(
.join_path(constants::array_enumerations_dir_name)
.join_path(enumeration_path);

auto&& tile = GenericTileIO::load(resources_, enmr_uri, 0, encryption_key);
resources_.get().stats().add_counter("read_enumeration_size", tile.size());
auto tile = GenericTileIO::load(resources_, enmr_uri, 0, encryption_key);
resources_.get().stats().add_counter("read_enumeration_size", tile->size());

if (!memory_tracker->take_memory(tile.size(), MemoryType::ENUMERATION)) {
if (!memory_tracker->take_memory(tile->size(), MemoryType::ENUMERATION)) {
throw ArrayDirectoryException(
"Error loading enumeration; Insufficient memory budget; Needed " +
std::to_string(tile.size()) + " but only had " +
std::to_string(tile->size()) + " but only had " +
std::to_string(memory_tracker->get_memory_available()) +
" from budget " + std::to_string(memory_tracker->get_memory_budget()));
}

Deserializer deserializer(tile.data(), tile.size());
Deserializer deserializer(tile->data(), tile->size());
return Enumeration::deserialize(deserializer);
}

Expand Down
13 changes: 6 additions & 7 deletions tiledb/sm/fragment/fragment_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ Status FragmentInfo::load_and_replace(
return Status::Ok();
}

tuple<Tile, std::vector<std::pair<std::string, uint64_t>>>
tuple<shared_ptr<Tile>, std::vector<std::pair<std::string, uint64_t>>>
load_consolidated_fragment_meta(
ContextResources& resources, const URI& uri, const EncryptionKey& enc_key) {
auto timer_se =
Expand All @@ -963,12 +963,12 @@ load_consolidated_fragment_meta(
throw StatusException(Status_FragmentInfoError(
"Cannot load consolidated fragment metadata; URI is empty."));

auto&& tile = GenericTileIO::load(resources, uri, 0, enc_key);
auto tile = GenericTileIO::load(resources, uri, 0, enc_key);

resources.stats().add_counter("consolidated_frag_meta_size", tile.size());
resources.stats().add_counter("consolidated_frag_meta_size", tile->size());

uint32_t fragment_num;
Deserializer deserializer(tile.data(), tile.size());
Deserializer deserializer(tile->data(), tile->size());
fragment_num = deserializer.read<uint32_t>();

uint64_t name_size, offset;
Expand All @@ -983,7 +983,7 @@ load_consolidated_fragment_meta(
ret.emplace_back(name, offset);
}

return {std::move(tile), std::move(ret)};
return {tile, std::move(ret)};
}

std::tuple<
Expand Down Expand Up @@ -1021,8 +1021,7 @@ FragmentInfo::load_array_schemas_and_fragment_metadata(
parallel_for(&resources.compute_tp(), 0, meta_uris.size(), [&](size_t i) {
auto&& [tile_opt, offsets] =
load_consolidated_fragment_meta(resources, meta_uris[i], enc_key);
fragment_metadata_tiles[i] =
make_shared<Tile>(HERE(), std::move(tile_opt));
fragment_metadata_tiles[i] = tile_opt;
offsets_vectors[i] = std::move(offsets);
return Status::Ok();
}));
Expand Down
Loading

0 comments on commit 05cf973

Please sign in to comment.