Skip to content

Commit 6561589

Browse files
authored
Remove move from Tile. (#4749)
This will help the memory tracking effort by removing all move constructors/assign operator for Tile, ResultTile, etc. [SC-41591] --- TYPE: NO_HISTORY DESC: Remove move from Tile.
1 parent 2f8188e commit 6561589

21 files changed

+155
-370
lines changed

test/src/unit-Reader.cc

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -278,38 +278,27 @@ TEST_CASE_METHOD(
278278
result_space_tiles);
279279
CHECK(result_space_tiles.size() == 6);
280280

281-
// Result tiles for fragment #1
282-
ResultTile result_tile_1_0_1(1, 0, *fragments[0]);
283-
ResultTile result_tile_1_2_1(1, 2, *fragments[0]);
284-
285-
// Result tiles for fragment #2
286-
ResultTile result_tile_1_0_2(2, 0, *fragments[1]);
287-
288-
// Result tiles for fragment #3
289-
ResultTile result_tile_2_0_3(3, 0, *fragments[2]);
290-
ResultTile result_tile_3_0_3(3, 2, *fragments[2]);
291-
292281
// Initialize result_space_tiles
293282
ResultSpaceTile<int32_t> rst_1_0;
294283
rst_1_0.set_start_coords({3, 1});
295284
rst_1_0.append_frag_domain(2, ds2);
296285
rst_1_0.append_frag_domain(1, ds1);
297-
rst_1_0.set_result_tile(1, result_tile_1_0_1);
298-
rst_1_0.set_result_tile(2, result_tile_1_0_2);
286+
rst_1_0.set_result_tile(1, 0, *fragments[0]);
287+
rst_1_0.set_result_tile(2, 0, *fragments[1]);
299288
ResultSpaceTile<int32_t> rst_1_2;
300289
rst_1_2.set_start_coords({3, 11});
301290
rst_1_2.append_frag_domain(1, ds1);
302-
rst_1_2.set_result_tile(1, result_tile_1_2_1);
291+
rst_1_2.set_result_tile(1, 2, *fragments[0]);
303292
ResultSpaceTile<int32_t> rst_2_0;
304293
rst_2_0.set_start_coords({5, 1});
305294
rst_2_0.append_frag_domain(3, ds3);
306-
rst_2_0.set_result_tile(3, result_tile_2_0_3);
295+
rst_2_0.set_result_tile(3, 0, *fragments[2]);
307296
ResultSpaceTile<int32_t> rst_2_2;
308297
rst_2_2.set_start_coords({5, 11});
309298
ResultSpaceTile<int32_t> rst_3_0;
310299
rst_3_0.set_start_coords({7, 1});
311300
rst_3_0.append_frag_domain(3, ds3);
312-
rst_3_0.set_result_tile(3, result_tile_3_0_3);
301+
rst_3_0.set_result_tile(3, 2, *fragments[2]);
313302
ResultSpaceTile<int32_t> rst_3_2;
314303
rst_3_2.set_start_coords({7, 11});
315304

test/src/unit-filter-pipeline.cc

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,14 @@ WriterTile make_offsets_tile(std::vector<uint64_t>& offsets) {
103103
}
104104

105105
Tile create_tile_for_unfiltering(uint64_t nelts, WriterTile& tile) {
106-
Tile ret(
106+
return {
107107
tile.format_version(),
108108
tile.type(),
109109
tile.cell_size(),
110110
0,
111111
tile.cell_size() * nelts,
112112
tile.filtered_buffer().data(),
113-
tile.filtered_buffer().size());
114-
return ret;
113+
tile.filtered_buffer().size()};
115114
}
116115

117116
void run_reverse(
@@ -1221,19 +1220,19 @@ TEST_CASE("Filter: Test encryption", "[filter][encryption]") {
12211220
key[0]++;
12221221
filter->set_key(key);
12231222

1224-
unfiltered_tile = create_tile_for_unfiltering(nelts, tile);
1225-
run_reverse(config, tp, unfiltered_tile, pipeline, false);
1223+
auto unfiltered_tile2 = create_tile_for_unfiltering(nelts, tile);
1224+
run_reverse(config, tp, unfiltered_tile2, pipeline, false);
12261225

12271226
// Fix key and check success.
1228-
unfiltered_tile = create_tile_for_unfiltering(nelts, tile);
1227+
auto unfiltered_tile3 = create_tile_for_unfiltering(nelts, tile);
12291228
key[0]--;
12301229
filter->set_key(key);
1231-
run_reverse(config, tp, unfiltered_tile, pipeline);
1230+
run_reverse(config, tp, unfiltered_tile3, pipeline);
12321231

12331232
for (uint64_t i = 0; i < nelts; i++) {
12341233
uint64_t elt = 0;
12351234
CHECK_NOTHROW(
1236-
unfiltered_tile.read(&elt, i * sizeof(uint64_t), sizeof(uint64_t)));
1235+
unfiltered_tile3.read(&elt, i * sizeof(uint64_t), sizeof(uint64_t)));
12371236
CHECK(elt == i);
12381237
}
12391238
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,7 +1527,7 @@ TEST_CASE_METHOD(
15271527
auto&& [array, fragments] = open_default_array_1d_with_fragments(capacity);
15281528

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

@@ -1540,8 +1540,9 @@ TEST_CASE_METHOD(
15401540

15411541
// Create the result_tiles pointer vector.
15421542
std::vector<ResultTile*> result_tiles(rt.size());
1543-
for (uint64_t i = 0; i < rt.size(); i++) {
1544-
result_tiles[i] = &rt[i];
1543+
uint64_t i = 0;
1544+
for (auto& t : rt) {
1545+
result_tiles[i++] = &t;
15451546
}
15461547

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

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

@@ -1756,8 +1757,9 @@ TEST_CASE_METHOD(
17561757

17571758
// Create the result_tiles pointer vector.
17581759
std::vector<ResultTile*> result_tiles(rt.size());
1759-
for (uint64_t i = 0; i < rt.size(); i++) {
1760-
result_tiles[i] = &rt[i];
1760+
uint64_t i = 0;
1761+
for (auto& t : rt) {
1762+
result_tiles[i++] = &t;
17611763
}
17621764

17631765
// Call the function.

tiledb/sm/array/array.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,10 +1478,8 @@ void Array::do_load_metadata() {
14781478
throw_if_not_ok(
14791479
parallel_for(&resources_.compute_tp(), 0, metadata_num, [&](size_t m) {
14801480
const auto& uri = array_metadata_to_load[m].uri_;
1481-
1482-
auto&& tile =
1481+
metadata_tiles[m] =
14831482
GenericTileIO::load(resources_, uri, 0, *encryption_key());
1484-
metadata_tiles[m] = tdb::make_shared<Tile>(HERE(), std::move(tile));
14851483

14861484
return Status::Ok();
14871485
}));

tiledb/sm/array/array_directory.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ shared_ptr<ArraySchema> ArrayDirectory::load_array_schema_from_uri(
9696
auto timer_se =
9797
resources.stats().start_timer("sm_load_array_schema_from_uri");
9898

99-
auto&& tile = GenericTileIO::load(resources, schema_uri, 0, encryption_key);
99+
auto tile = GenericTileIO::load(resources, schema_uri, 0, encryption_key);
100100

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

103103
// Deserialize
104-
Deserializer deserializer(tile.data(), tile.size());
104+
Deserializer deserializer(tile->data(), tile->size());
105105
auto memory_tracker = resources.create_memory_tracker();
106106
memory_tracker->set_type(MemoryTrackerType::ARRAY_LOAD);
107107
return ArraySchema::deserialize(deserializer, schema_uri, memory_tracker);
@@ -1323,18 +1323,18 @@ shared_ptr<const Enumeration> ArrayDirectory::load_enumeration(
13231323
.join_path(constants::array_enumerations_dir_name)
13241324
.join_path(enumeration_path);
13251325

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

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

1337-
Deserializer deserializer(tile.data(), tile.size());
1337+
Deserializer deserializer(tile->data(), tile->size());
13381338
return Enumeration::deserialize(deserializer);
13391339
}
13401340

tiledb/sm/fragment/fragment_info.cc

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ Status FragmentInfo::load_and_replace(
952952
return Status::Ok();
953953
}
954954

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

966-
auto&& tile = GenericTileIO::load(resources, uri, 0, enc_key);
966+
auto tile = GenericTileIO::load(resources, uri, 0, enc_key);
967967

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

970970
uint32_t fragment_num;
971-
Deserializer deserializer(tile.data(), tile.size());
971+
Deserializer deserializer(tile->data(), tile->size());
972972
fragment_num = deserializer.read<uint32_t>();
973973

974974
uint64_t name_size, offset;
@@ -983,7 +983,7 @@ load_consolidated_fragment_meta(
983983
ret.emplace_back(name, offset);
984984
}
985985

986-
return {std::move(tile), std::move(ret)};
986+
return {tile, std::move(ret)};
987987
}
988988

989989
std::tuple<
@@ -1021,8 +1021,7 @@ FragmentInfo::load_array_schemas_and_fragment_metadata(
10211021
parallel_for(&resources.compute_tp(), 0, meta_uris.size(), [&](size_t i) {
10221022
auto&& [tile_opt, offsets] =
10231023
load_consolidated_fragment_meta(resources, meta_uris[i], enc_key);
1024-
fragment_metadata_tiles[i] =
1025-
make_shared<Tile>(HERE(), std::move(tile_opt));
1024+
fragment_metadata_tiles[i] = tile_opt;
10261025
offsets_vectors[i] = std::move(offsets);
10271026
return Status::Ok();
10281027
}));

0 commit comments

Comments
 (0)