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.

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

Cpp metrics plugin #638

Merged
merged 14 commits into from
Nov 8, 2023
Merged

Cpp metrics plugin #638

merged 14 commits into from
Nov 8, 2023

Conversation

intjftw
Copy link
Collaborator

@intjftw intjftw commented Oct 3, 2023

This is the basis of the C++ metrics plugin. It contains the usual components of a plugin, including the fundamental db tables to store module-level and AST node-level (function, class) metrics. This plugin is intended to help Software Technology lab students start their projects.

@intjftw intjftw added Kind: Enhancement 🌟 Status: WIP 👷 Issue or PR under development - feel free to review, though! labels Oct 3, 2023
@intjftw intjftw added this to the Release Gershwin milestone Oct 3, 2023
@intjftw intjftw requested review from zporky and mcserep October 3, 2023 12:07
Copy link
Collaborator

@mcserep mcserep left a comment

Choose a reason for hiding this comment

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

Made a quick initial review based on the code.

plugins/cpp_metrics/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/cpp_metrics/service/src/plugin.cpp Outdated Show resolved Hide resolved
plugins/cpp_metrics/service/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/cpp_metrics/parser/src/cppmetricsparser.cpp Outdated Show resolved Hide resolved
plugins/cpp_metrics/parser/src/cppmetricsparser.cpp Outdated Show resolved Hide resolved
@mcserep mcserep removed this from the Release Gershwin milestone Oct 3, 2023
@mcserep mcserep marked this pull request as draft October 3, 2023 14:58
@mcserep mcserep marked this pull request as ready for review October 3, 2023 23:39
CMakeLists.txt Show resolved Hide resolved
@intjftw intjftw removed the Status: WIP 👷 Issue or PR under development - feel free to review, though! label Oct 4, 2023
@mcserep mcserep added Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin. labels Oct 4, 2023
Copy link
Collaborator

@mcserep mcserep left a comment

Choose a reason for hiding this comment

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

Looks good with the changes 👏

@mcserep
Copy link
Collaborator

mcserep commented Oct 11, 2023

Incremental parsing should be supported by the new plugin as well.
The workflow for incremental parsing is the following (in case of an initial or forced parsing, only step 5 is executed):

  1. directly modified files are detected by ParserContext.
  2. all plugin parsers mark the indirectly modified files.
  3. all plugin parsers perform a cleanup operation.
  4. global tables are cleaned up by parser/src/parser.cpp.
  5. all plugin parsers perform a parsing operation.

Possible solutions:

  • For simple metrics without external relation, like the function argument count metric, a simple cleanup could be performed to remove all entries from the database, which belongs to modified (or deleted) files. Then only these files needs to be reparsed.
  • Alternatively, a complete cleanup of all C++ metrics could be performed, not supporting incremental parsing for this plugin initially, but instead enforcing a complete recalculation of all metrics.

Comment on lines +66 to +82
for (const auto& pair : _astNodeIdCache)
{
auto file = _ctx.db->query_one<model::File>(
odb::query<model::File>::id == pair.second);

auto it = _ctx.fileStatus.find(file->path);
if (it != _ctx.fileStatus.end() &&
(it->second == cc::parser::IncrementalStatus::DELETED ||
it->second == cc::parser::IncrementalStatus::MODIFIED ||
it->second == cc::parser::IncrementalStatus::ACTION_CHANGED))
{
LOG(info) << "[cxxmetricsparser] Database cleanup: " << file->path;

_ctx.db->erase_query<model::CppAstNodeMetrics>(odb::query<model::CppAstNodeMetrics>::astNodeId == pair.first);
_astNodeIdCache.erase(pair.first);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the loop above you are already iterating through all files and purging the ones, which has deleted, modified or action changed status. There is no need for another loop or for the _astNodeIdCache at all, we could simply purge the AST nodes there as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will accept it like this, so we can merge this, and created #670.

@mcserep mcserep merged commit 9c4a016 into Ericsson:master Nov 8, 2023
7 checks passed
wbqpk3 pushed a commit to wbqpk3/CodeCompass that referenced this pull request Jul 8, 2024
Co-authored-by: Máté Cserép <mcserep@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Enhancement 🌟 Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin.
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants