Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove serialization non C41 constructors from stats #4706

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Remove serialization non C41 constructors from stats #4706

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5590ded
Remove serialization non C41 constructors from stats
robertbindar Feb 8, 2024
c1e5b58
address review comments
robertbindar Feb 12, 2024
075ca36
Small doxygen fixes.
KiterLuc Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions tiledb/sm/query/query.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ Status Query::process() {

if (!only_dim_label_query()) {
if (strategy_ != nullptr) {
// The strategy destructor should reset its own Stats object here
dynamic_cast<StrategyBase*>(strategy_.get())->stats()->reset();
strategy_ = nullptr;
}
Expand Down Expand Up @@ -917,6 +918,7 @@ Status Query::reset_strategy_with_layout(
Layout layout, bool force_legacy_reader) {
force_legacy_reader_ = force_legacy_reader;
if (strategy_ != nullptr) {
// The strategy destructor should reset its own Stats object here
dynamic_cast<StrategyBase*>(strategy_.get())->stats()->reset();
strategy_ = nullptr;
}
Expand Down Expand Up @@ -1663,6 +1665,10 @@ stats::Stats* Query::stats() const {
return stats_;
}

void Query::set_stats(const stats::StatsData& data) {
stats_->populate_with_data(data);
}

shared_ptr<Buffer> Query::rest_scratch() const {
return rest_scratch_;
}
Expand Down
6 changes: 6 additions & 0 deletions tiledb/sm/query/query.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,12 @@ class Query {
/** Returns the internal stats object. */
stats::Stats* stats() const;

/**
* Populate the owned stats instance with data.
* To be removed when the class will get a C41 constructor.
KiterLuc marked this conversation as resolved.
Show resolved Hide resolved
*/
void set_stats(const stats::StatsData& data);

/** Returns the scratch space used for REST requests. */
shared_ptr<Buffer> rest_scratch() const;

Expand Down
4 changes: 4 additions & 0 deletions tiledb/sm/query/strategy_base.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ stats::Stats* StrategyBase::stats() const {
return stats_;
}

void StrategyBase::set_stats(const stats::StatsData& data) {
stats_->populate_with_data(data);
}

/* ****************************** */
/* PROTECTED METHODS */
/* ****************************** */
Expand Down
6 changes: 6 additions & 0 deletions tiledb/sm/query/strategy_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ class StrategyBase {
/** Returns `stats_`. */
stats::Stats* stats() const;

/**
* Populate the owned stats instance with data.
* To be removed when the class will get a C41 constructor.
KiterLuc marked this conversation as resolved.
Show resolved Hide resolved
*/
void set_stats(const stats::StatsData& data);

/** Returns the configured offsets format mode. */
std::string offsets_mode() const;

Expand Down
151 changes: 53 additions & 98 deletions tiledb/sm/serialization/query.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ namespace serialization {

shared_ptr<Logger> dummy_logger = make_shared<Logger>(HERE(), "");

Status stats_to_capnp(Stats& stats, capnp::Stats::Builder* stats_builder) {
void stats_to_capnp(const Stats& stats, capnp::Stats::Builder* stats_builder) {
// Build counters
const auto counters = stats.counters();
if (counters != nullptr && !counters->empty()) {
Expand All @@ -114,31 +114,29 @@ Status stats_to_capnp(Stats& stats, capnp::Stats::Builder* stats_builder) {
++index;
}
}

return Status::Ok();
}

Status stats_from_capnp(
const capnp::Stats::Reader& stats_reader, Stats* stats) {
StatsData stats_from_capnp(const capnp::Stats::Reader& stats_reader) {
std::unordered_map<std::string, uint64_t> counters;
std::unordered_map<std::string, double> timers;

if (stats_reader.hasCounters()) {
auto counters = stats->counters();
auto counters_reader = stats_reader.getCounters();
for (const auto entry : counters_reader.getEntries()) {
auto key = std::string_view{entry.getKey().cStr(), entry.getKey().size()};
(*counters)[std::string{key}] = entry.getValue();
counters[std::string(key)] = entry.getValue();
}
}

if (stats_reader.hasTimers()) {
auto timers = stats->timers();
auto timers_reader = stats_reader.getTimers();
for (const auto entry : timers_reader.getEntries()) {
auto key = std::string_view{entry.getKey().cStr(), entry.getKey().size()};
(*timers)[std::string{key}] = entry.getValue();
timers[std::string(key)] = entry.getValue();
}
}

return Status::Ok();
return stats::StatsData(counters, timers);
}

void range_buffers_to_capnp(
Expand Down Expand Up @@ -246,11 +244,9 @@ Status subarray_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = subarray->stats();
if (stats != nullptr) {
auto stats_builder = builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = subarray->stats();
auto stats_builder = builder->initStats();
stats_to_capnp(stats, &stats_builder);

if (subarray->relevant_fragments().relevant_fragments_size() > 0) {
auto relevant_fragments_builder = builder->initRelevantFragments(
Expand Down Expand Up @@ -328,11 +324,8 @@ Status subarray_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader.hasStats()) {
stats::Stats* stats = subarray->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader.getStats());
subarray->set_stats(stats_data);
}

if (reader.hasRelevantFragments()) {
Expand Down Expand Up @@ -428,11 +421,9 @@ Status subarray_partitioner_to_capnp(
builder->setMemoryBudgetValidity(mem_budget_validity);

// If stats object exists set its cap'n proto object
stats::Stats* stats = partitioner.stats();
if (stats != nullptr) {
auto stats_builder = builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = partitioner.stats();
auto stats_builder = builder->initStats();
stats_to_capnp(stats, &stats_builder);

return Status::Ok();
}
Expand Down Expand Up @@ -566,11 +557,8 @@ Status subarray_partitioner_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader.hasStats()) {
auto stats = partitioner->stats();
// We should always have stats
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader.getStats());
partitioner->set_stats(stats_data);
}

return Status::Ok();
Expand Down Expand Up @@ -913,11 +901,9 @@ Status reader_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = reader.stats();
if (stats != nullptr) {
auto stats_builder = reader_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *reader.stats();
auto stats_builder = reader_builder->initStats();
stats_to_capnp(stats, &stats_builder);

return Status::Ok();
}
Expand Down Expand Up @@ -947,11 +933,9 @@ Status index_reader_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = reader.stats();
if (stats != nullptr) {
auto stats_builder = reader_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *reader.stats();
auto stats_builder = reader_builder->initStats();
stats_to_capnp(stats, &stats_builder);

return Status::Ok();
}
Expand Down Expand Up @@ -982,11 +966,9 @@ Status dense_reader_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = reader.stats();
if (stats != nullptr) {
auto stats_builder = reader_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *reader.stats();
auto stats_builder = reader_builder->initStats();
stats_to_capnp(stats, &stats_builder);

return Status::Ok();
}
Expand Down Expand Up @@ -1146,11 +1128,8 @@ Status reader_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader_reader.hasStats()) {
stats::Stats* stats = reader->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(reader_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader_reader.getStats());
reader->set_stats(stats_data);
}

return Status::Ok();
Expand Down Expand Up @@ -1187,11 +1166,8 @@ Status index_reader_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader_reader.hasStats()) {
stats::Stats* stats = reader->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(reader_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader_reader.getStats());
reader->set_stats(stats_data);
}

return Status::Ok();
Expand Down Expand Up @@ -1229,11 +1205,8 @@ Status dense_reader_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader_reader.hasStats()) {
stats::Stats* stats = reader->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(reader_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader_reader.getStats());
reader->set_stats(stats_data);
}

return Status::Ok();
Expand All @@ -1252,11 +1225,8 @@ Status delete_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (delete_reader.hasStats()) {
stats::Stats* stats = delete_strategy->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(delete_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(delete_reader.getStats());
delete_strategy->set_stats(stats_data);
}

return Status::Ok();
Expand All @@ -1273,11 +1243,9 @@ Status delete_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = delete_strategy.stats();
if (stats != nullptr) {
auto stats_builder = delete_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *delete_strategy.stats();
auto stats_builder = delete_builder->initStats();
stats_to_capnp(stats, &stats_builder);

return Status::Ok();
}
Expand All @@ -1301,11 +1269,9 @@ Status writer_to_capnp(
}

// If stats object exists set its cap'n proto object
stats::Stats* stats = writer.stats();
if (stats != nullptr) {
auto stats_builder = writer_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *writer.stats();
auto stats_builder = writer_builder->initStats();
stats_to_capnp(stats, &stats_builder);

if (query.layout() == Layout::GLOBAL_ORDER) {
auto& global_writer = dynamic_cast<GlobalOrderWriter&>(writer);
Expand Down Expand Up @@ -1341,11 +1307,8 @@ Status writer_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (writer_reader.hasStats()) {
stats::Stats* stats = writer->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(writer_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(writer_reader.getStats());
writer->set_stats(stats_data);
}

if (query.layout() == Layout::GLOBAL_ORDER &&
Expand Down Expand Up @@ -1557,10 +1520,10 @@ Status query_to_capnp(
RETURN_NOT_OK(config_to_capnp(query.config(), &config_builder));

// If stats object exists set its cap'n proto object
stats::Stats* stats = query.stats();
if (stats != nullptr) {
auto stats = query.stats();
if (stats) {
auto stats_builder = query_builder->initStats();
RETURN_NOT_OK(stats_to_capnp(*stats, &stats_builder));
stats_to_capnp(*stats, &stats_builder);
}

auto& written_fragment_info = query.get_written_fragment_info();
Expand Down Expand Up @@ -2270,11 +2233,8 @@ Status query_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (query_reader.hasStats()) {
stats::Stats* stats = query->stats();
// We should always have a stats here
if (stats != nullptr) {
RETURN_NOT_OK(stats_from_capnp(query_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(query_reader.getStats());
query->set_stats(stats_data);
}

if (query_reader.hasWrittenFragmentInfo()) {
Expand Down Expand Up @@ -3180,11 +3140,9 @@ void ordered_dim_label_reader_to_capnp(
query.dimension_label_increasing_order());

// If stats object exists set its cap'n proto object
stats::Stats* stats = reader.stats();
if (stats != nullptr) {
auto stats_builder = reader_builder->initStats();
throw_if_not_ok(stats_to_capnp(*stats, &stats_builder));
}
const auto& stats = *reader.stats();
auto stats_builder = reader_builder->initStats();
stats_to_capnp(stats, &stats_builder);
}

void ordered_dim_label_reader_from_capnp(
Expand Down Expand Up @@ -3212,11 +3170,8 @@ void ordered_dim_label_reader_from_capnp(

// If cap'n proto object has stats set it on c++ object
if (reader_reader.hasStats()) {
stats::Stats* stats = reader->stats();
// We should always have a stats here
if (stats != nullptr) {
throw_if_not_ok(stats_from_capnp(reader_reader.getStats(), stats));
}
auto stats_data = stats_from_capnp(reader_reader.getStats());
reader->set_stats(stats_data);
}
}

Expand Down
Loading
Loading