Skip to content

Query paginated metrics efficiently #804

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

Merged
mcserep merged 1 commit into from
Jul 25, 2025
Merged

Query paginated metrics efficiently #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 3 additions & 3 deletions plugins/cpp_metrics/service/cxxmetrics.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ service CppMetricsService
map<common.AstNodeId, list<CppMetricsAstNodeSingle>> getPagedCppAstNodeMetricsForPath(
1:string path
2:i32 pageSize,
3:i32 pageNumber)
3:common.AstNodeId previousId)

/**
* This function returns all available C++ metrics
Expand All @@ -129,7 +129,7 @@ service CppMetricsService
map<common.AstNodeId, CppMetricsAstNodeDetailed> getPagedCppAstNodeMetricsDetailedForPath(
1:string path,
2:i32 pageSize,
3:i32 pageNumber)
3:common.AstNodeId previousId)

/**
* This function returns all available C++ metrics
Expand All @@ -149,7 +149,7 @@ service CppMetricsService
map<common.FileId, list<CppMetricsModuleSingle>> getPagedCppFileMetricsForPath(
1:string path,
2:i32 pageSize,
3:i32 pageNumber)
3:common.FileId previousId)

/**
* This function returns the names of the AST node-level C++ metrics.
Expand Down
36 changes: 10 additions & 26 deletions plugins/cpp_metrics/service/include/service/cppmetricsservice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf
std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_) override;
const core::AstNodeId& previousId_) override;

void getCppAstNodeMetricsDetailedForPath(
std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
Expand All @@ -67,7 +67,7 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf
std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_) override;
const core::AstNodeId& previousId_) override;

void getCppFileMetricsForPath(
std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
Expand All @@ -77,7 +77,7 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf
std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_) override;
const core::FileId& previousId_) override;

void getCppAstNodeMetricsTypeNames(
std::vector<CppAstNodeMetricsTypeName>& _return) override;
Expand All @@ -91,33 +91,17 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf

const boost::program_options::variables_map& _config;

std::string getPagingQuery(
std::string getLimitQuery(const std::int32_t pageSize_);

std::vector<model::CppAstNodeId> pageAstNodeMetrics(
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_);
const model::CppAstNodeId previousId_);

template <typename TID, typename TView>
std::vector<TID> pageMetrics(
std::vector<model::FileId> pageFileMetrics(
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_)
{
return _transaction([&, this](){
odb::result<TView> paged_nodes = _db->query<TView>(
odb::query<TView>::File::path.like(path_ + '%') + getPagingQuery(pageSize_, pageNumber_));

std::vector<TID> paged_ids(paged_nodes.size());
std::transform(paged_nodes.begin(), paged_nodes.end(), paged_ids.begin(),
[](const TView& e){
if constexpr (std::is_same<TView, model::CppAstNodeMetricsDistinctView>::value) {
return e.astNodeId;
} else if constexpr (std::is_same<TView, model::CppModuleMetricsDistinctView>::value) {
return e.fileId;
}
});

return paged_ids;
});
}
const model::FileId previousId_);

void queryCppAstNodeMetricsForPath(
std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
Expand Down
88 changes: 69 additions & 19 deletions plugins/cpp_metrics/service/src/cppmetricsservice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ void CppMetricsServiceHandler::getPagedCppAstNodeMetricsForPath(
std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_)
const core::AstNodeId& previousId_)
{
std::vector<model::CppAstNodeId> paged_nodes = pageMetrics<model::CppAstNodeId, model::CppAstNodeMetricsDistinctView>(
path_, pageSize_, pageNumber_);
std::vector<model::CppAstNodeId> paged_nodes = pageAstNodeMetrics(
path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std::stoull function can throw std::invalid_argument or std::out_of_range exceptions when the string cannot be converted to an unsigned long long. Consider adding proper error handling or validation for the previousId_ parameter.

Copilot uses AI. Check for mistakes.

queryCppAstNodeMetricsForPath(_return,
CppNodeMetricsQuery::CppAstNodeMetrics::astNodeId.in_range(paged_nodes.begin(), paged_nodes.end()));
Expand Down Expand Up @@ -241,10 +241,10 @@ void CppMetricsServiceHandler::getPagedCppAstNodeMetricsDetailedForPath(
std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_)
const core::AstNodeId& previousId_)
{
std::vector<model::CppAstNodeId> paged_nodes = pageMetrics<model::CppAstNodeId, model::CppAstNodeMetricsDistinctView>(
path_, pageSize_, pageNumber_);
std::vector<model::CppAstNodeId> paged_nodes = pageAstNodeMetrics(
path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std::stoull function can throw std::invalid_argument or std::out_of_range exceptions when the string cannot be converted to an unsigned long long. Consider adding proper error handling or validation for the previousId_ parameter.

Copilot uses AI. Check for mistakes.

queryCppAstNodeMetricsDetailedForPath(_return,
CppNodeMetricsQuery::CppAstNodeMetrics::astNodeId.in_range(paged_nodes.begin(), paged_nodes.end()));
Expand Down Expand Up @@ -290,16 +290,16 @@ void CppMetricsServiceHandler::getPagedCppFileMetricsForPath(
std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
const std::string& path_,
const std::int32_t pageSize_,
const std::int32_t pageNumber_)
const core::FileId& previousId_)
{
std::vector<model::FileId> paged_files = pageMetrics<model::FileId, model::CppModuleMetricsDistinctView>(path_, pageSize_, pageNumber_);
std::vector<model::FileId> paged_files = pageFileMetrics(
path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
Comment on lines +295 to +296
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std::stoull function can throw std::invalid_argument or std::out_of_range exceptions when the string cannot be converted to an unsigned long long. Consider adding proper error handling or validation for the previousId_ parameter.

Suggested change
std::vector<model::FileId> paged_files = pageFileMetrics(
path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
std::uint64_t previousIdValue = 0;
if (!previousId_.empty()) {
try {
previousIdValue = std::stoull(previousId_);
} catch (const std::invalid_argument& e) {
core::InvalidInput ex;
ex.__set_msg("Invalid previousId_: " + previousId_ + ". Must be a numeric value.");
throw ex;
} catch (const std::out_of_range& e) {
core::InvalidInput ex;
ex.__set_msg("Invalid previousId_: " + previousId_ + ". Value out of range.");
throw ex;
}
}
std::vector<model::FileId> paged_files = pageFileMetrics(path_, pageSize_, previousIdValue);

Copilot uses AI. Check for mistakes.
queryCppFileMetricsForPath(_return,
CppModuleMetricsQuery::CppFileMetrics::file.in_range(paged_files.begin(), paged_files.end()));
}

std::string CppMetricsServiceHandler::getPagingQuery(
const std::int32_t pageSize_,
const std::int32_t pageNumber_)
std::string CppMetricsServiceHandler::getLimitQuery(
const std::int32_t pageSize_)
{
if (pageSize_ <= 0)
{
Expand All @@ -308,15 +308,65 @@ std::string CppMetricsServiceHandler::getPagingQuery(
throw ex;
}

if (pageNumber_ <= 0)
{
core::InvalidInput ex;
ex.__set_msg("Invalid page number: " + std::to_string(pageNumber_));
throw ex;
}
return " LIMIT " + std::to_string(pageSize_);
}

std::vector<model::CppAstNodeId> CppMetricsServiceHandler::pageAstNodeMetrics(
const std::string& path_,
const std::int32_t pageSize_,
const model::CppAstNodeId previousId_)
{
typedef odb::query<model::CppAstNodeMetricsDistinctView> MetricQuery;
typedef odb::result<model::CppAstNodeMetricsDistinctView> MetricResult;

return _transaction([&, this](){
MetricQuery condition = MetricQuery::File::path.like(path_ + '%');
if (previousId_ != 0) {
condition = condition && (MetricQuery::CppAstNodeMetrics::astNodeId > previousId_);
}

MetricResult paged_nodes = _db->query<model::CppAstNodeMetricsDistinctView>(
condition +
("ORDER BY" + odb::query<model::CppAstNodeMetrics>::astNodeId) +
getLimitQuery(pageSize_));

const std::int32_t offset = (pageNumber_ - 1) * pageSize_;
return " LIMIT " + std::to_string(pageSize_) + " OFFSET " + std::to_string(offset);
std::vector<model::CppAstNodeId> paged_ids(paged_nodes.size());
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-sizing the vector with paged_nodes.size() may not be accurate since the result set size might differ from the allocated size. Consider using reserve() instead of sizing the constructor, or use emplace_back() in the transform operation.

Copilot uses AI. Check for mistakes.
std::transform(paged_nodes.begin(), paged_nodes.end(), paged_ids.begin(),
[](const model::CppAstNodeMetricsDistinctView& e){
return e.astNodeId;
});

return paged_ids;
});
}

std::vector<model::FileId> CppMetricsServiceHandler::pageFileMetrics(
const std::string& path_,
const std::int32_t pageSize_,
const model::FileId previousId_)
{
typedef odb::query<model::CppModuleMetricsDistinctView> MetricQuery;
typedef odb::result<model::CppModuleMetricsDistinctView> MetricResult;

return _transaction([&, this](){
MetricQuery condition = MetricQuery::File::path.like(path_ + '%');
if (previousId_ != 0) {
condition = condition && (MetricQuery::CppFileMetrics::file > previousId_);
}

MetricResult paged_nodes = _db->query<model::CppModuleMetricsDistinctView>(
condition +
("ORDER BY" + odb::query<model::CppFileMetrics>::file) +
getLimitQuery(pageSize_));

std::vector<model::FileId> paged_ids(paged_nodes.size());
std::transform(paged_nodes.begin(), paged_nodes.end(), paged_ids.begin(),
[](const model::CppModuleMetricsDistinctView& e){
return e.fileId;
});
Comment on lines +362 to +366
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-sizing the vector with paged_nodes.size() may not be accurate since the result set size might differ from the allocated size. Consider using reserve() instead of sizing the constructor, or use emplace_back() in the transform operation.

Suggested change
std::vector<model::FileId> paged_ids(paged_nodes.size());
std::transform(paged_nodes.begin(), paged_nodes.end(), paged_ids.begin(),
[](const model::CppModuleMetricsDistinctView& e){
return e.fileId;
});
std::vector<model::FileId> paged_ids;
paged_ids.reserve(paged_nodes.size());
for (const auto& e : paged_nodes) {
paged_ids.emplace_back(e.fileId);
}

Copilot uses AI. Check for mistakes.

return paged_ids;
});
}

} // cppmetrics
Expand Down
Loading