Skip to content

Commit 1d40d5c

Browse files
committed
Address most review comments
1 parent 96e660f commit 1d40d5c

File tree

5 files changed

+23
-36
lines changed

5 files changed

+23
-36
lines changed

test/src/unit-sparse-global-order-reader.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -789,9 +789,10 @@ TEST_CASE_METHOD(
789789
write_1d_fragment(coords, &coords_size, data, &data_size);
790790
}
791791

792-
// Two result tiles (2 * (~4500 + 8) will be bigger than the per fragment
793-
// budget (3905).
794-
total_budget_ = "60000";
792+
// Two result tiles (2 * (2736 + 8)) = 5488 will be bigger than the per
793+
// fragment budget (50000 * 0.11 / 2 fragments = 2750), so only one result
794+
// tile will be loaded each time.
795+
total_budget_ = "50000";
795796
ratio_coords_ = "0.11";
796797
update_config();
797798

@@ -1312,8 +1313,9 @@ TEST_CASE_METHOD(
13121313
write_1d_fragment(coords, &coords_size, data, &data_size);
13131314
}
13141315

1315-
// Two result tile (2 * (~4500 + 8) will be bigger than the per fragment
1316-
// budget (1000).
1316+
// Two result tiles (2 * (2736 + 8)) = 5488 will be bigger than the per
1317+
// fragment budget (40000 * 0.22 /2 frag = 4400), so only one will be loaded
1318+
// each time.
13171319
total_budget_ = "40000";
13181320
ratio_coords_ = "0.22";
13191321
update_config();

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,10 +1066,9 @@ TEST_CASE_METHOD(
10661066
CHECK(1 == loop_num->second);
10671067
}
10681068
/**
1069-
* FIXME: The loop_num appears to be different on different
1070-
* architectures/build modes. SC-61065 to investigate why. } else { CHECK(20
1071-
* == loop_num->second);
1072-
* }
1069+
* We can't do a similar check for multiple fragments as it is architecture
1070+
* dependent how many tiles fit in the memory budget. And thus also
1071+
* architecture dependent as to how many internal loops we have.
10731072
*/
10741073

10751074
// Try to read multiple frags without partial tile offset reading. Should

tiledb/sm/filter/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ include(object_library)
3232
#
3333
commence(object_library filter)
3434
this_target_sources(filter.cc filter_buffer.cc filter_storage.cc)
35-
this_target_object_libraries(baseline buffer tiledb_crypto thread_pool)
35+
this_target_object_libraries(baseline buffer tiledb_crypto)
3636
conclude(object_library)
3737

3838
#

tiledb/sm/query/readers/result_tile.cc

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,14 @@ ResultTile::ResultTile(
9393

9494
ResultTile::~ResultTile() {
9595
try {
96-
// Wait for all tasks to be done
97-
wait_all_attrs();
96+
// Wait for all attribute tasks to be done
97+
wait_all_tiles(attr_tiles_);
9898
} catch (...) {
9999
}
100100

101101
try {
102-
// Wait for all tasks to be done
103-
wait_all_coords();
102+
// Wait for all coordinates tasks to be done
103+
wait_all_tiles(coord_tiles_);
104104
} catch (...) {
105105
}
106106
}
@@ -275,23 +275,10 @@ ResultTile::TileTuple* ResultTile::tile_tuple(const std::string& name) {
275275
return nullptr;
276276
}
277277

278-
void ResultTile::wait_all_coords() const {
279-
for (auto& at : coord_tiles_) {
280-
auto& tile_tuple = at.second;
281-
if (tile_tuple.has_value()) {
282-
tile_tuple.value().fixed_tile().data();
283-
if (tile_tuple.value().var_tile_opt().has_value()) {
284-
tile_tuple.value().var_tile_opt().value().data();
285-
}
286-
if (tile_tuple.value().validity_tile_opt().has_value()) {
287-
tile_tuple.value().validity_tile_opt().value().data();
288-
}
289-
}
290-
}
291-
}
292-
293-
void ResultTile::wait_all_attrs() const {
294-
for (auto& at : attr_tiles_) {
278+
void ResultTile::wait_all_tiles(
279+
const std::vector<std::pair<std::string, optional<TileTuple>>>& tiles)
280+
const {
281+
for (auto& at : tiles) {
295282
const auto& tile_tuple = at.second;
296283
if (tile_tuple.has_value()) {
297284
tile_tuple.value().fixed_tile().data();

tiledb/sm/query/readers/result_tile.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -742,11 +742,10 @@ class ResultTile {
742742
const uint64_t min_cell,
743743
const uint64_t max_cell) const;
744744

745-
/* Waits for all coord tiles results to be available */
746-
void wait_all_coords() const;
747-
748-
/* Waits for all attr tiles results to be available */
749-
void wait_all_attrs() const;
745+
/* Waits for all tiles results to be available */
746+
void wait_all_tiles(
747+
const std::vector<std::pair<std::string, optional<TileTuple>>>& tiles)
748+
const;
750749

751750
protected:
752751
/* ********************************* */

0 commit comments

Comments
 (0)