Skip to content

Commit ba1cfb5

Browse files
committed
Remove move from Tile.
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.
1 parent a84789e commit ba1cfb5

21 files changed

+155
-370
lines changed

test/src/unit-Reader.cc

+5-16
Original file line numberDiff line numberDiff line change
@@ -276,38 +276,27 @@ TEST_CASE_METHOD(
276276
result_space_tiles);
277277
CHECK(result_space_tiles.size() == 6);
278278

279-
// Result tiles for fragment #1
280-
ResultTile result_tile_1_0_1(1, 0, *fragments[0]);
281-
ResultTile result_tile_1_2_1(1, 2, *fragments[0]);
282-
283-
// Result tiles for fragment #2
284-
ResultTile result_tile_1_0_2(2, 0, *fragments[1]);
285-
286-
// Result tiles for fragment #3
287-
ResultTile result_tile_2_0_3(3, 0, *fragments[2]);
288-
ResultTile result_tile_3_0_3(3, 2, *fragments[2]);
289-
290279
// Initialize result_space_tiles
291280
ResultSpaceTile<int32_t> rst_1_0;
292281
rst_1_0.set_start_coords({3, 1});
293282
rst_1_0.append_frag_domain(2, ds2);
294283
rst_1_0.append_frag_domain(1, ds1);
295-
rst_1_0.set_result_tile(1, result_tile_1_0_1);
296-
rst_1_0.set_result_tile(2, result_tile_1_0_2);
284+
rst_1_0.set_result_tile(1, 0, *fragments[0]);
285+
rst_1_0.set_result_tile(2, 0, *fragments[1]);
297286
ResultSpaceTile<int32_t> rst_1_2;
298287
rst_1_2.set_start_coords({3, 11});
299288
rst_1_2.append_frag_domain(1, ds1);
300-
rst_1_2.set_result_tile(1, result_tile_1_2_1);
289+
rst_1_2.set_result_tile(1, 2, *fragments[0]);
301290
ResultSpaceTile<int32_t> rst_2_0;
302291
rst_2_0.set_start_coords({5, 1});
303292
rst_2_0.append_frag_domain(3, ds3);
304-
rst_2_0.set_result_tile(3, result_tile_2_0_3);
293+
rst_2_0.set_result_tile(3, 0, *fragments[2]);
305294
ResultSpaceTile<int32_t> rst_2_2;
306295
rst_2_2.set_start_coords({5, 11});
307296
ResultSpaceTile<int32_t> rst_3_0;
308297
rst_3_0.set_start_coords({7, 1});
309298
rst_3_0.append_frag_domain(3, ds3);
310-
rst_3_0.set_result_tile(3, result_tile_3_0_3);
299+
rst_3_0.set_result_tile(3, 2, *fragments[2]);
311300
ResultSpaceTile<int32_t> rst_3_2;
312301
rst_3_2.set_start_coords({7, 11});
313302

test/src/unit-filter-pipeline.cc

+7-8
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

+8-6
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

+1-3
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

+8-8
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
return make_shared<ArraySchema>(
106106
HERE(), ArraySchema::deserialize(deserializer, schema_uri));
107107
}
@@ -1322,18 +1322,18 @@ shared_ptr<const Enumeration> ArrayDirectory::load_enumeration(
13221322
.join_path(constants::array_enumerations_dir_name)
13231323
.join_path(enumeration_path);
13241324

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

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

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

tiledb/sm/fragment/fragment_info.cc

+6-7
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)