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

Added missing debug output to a variety of operations. #174

Merged
merged 2 commits into from Jan 25, 2019

Conversation

floriankramer
Copy link
Member

This pr adds debug logging to the beginning and end of all operations written by me (which where so far mostly lacking in any output in the logs.

@niklas88
Copy link
Member

Independent from adding this set of debug information which I think is definitely a good idea, I
just talked with @hannahbast about useful information to get from the pattern trick. She says we probably want to get the following metrics:

  • How many patterns were found vs number of entities (raw numbers + percentage)
  • How many predicates came from patterns vs total predicates (raw numbers + percentage)
  • How many "conceptual operations" did we use vs the baseline of aggregating raw predicates

Copy link
Member

@niklas88 niklas88 left a comment

Choose a reason for hiding this comment

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

LGTM though I would change the wording for the "Beginning" and "Finished" messages. Also I've added #178 to track the other statistics so we can merge these simpler messages before that.

@@ -108,6 +108,8 @@ size_t CountAvailablePredicates::getCostEstimate() {

// _____________________________________________________________________________
void CountAvailablePredicates::computeResult(ResultTable* result) const {
LOG(DEBUG) << "Beginning with computing the CountAvailablePredicates result."
Copy link
Member

Choose a reason for hiding this comment

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

"Begin computing …" sounds a bit weird to me. I think what that's because "-ing" words are normally for stuff that that takes time and it clashes with "the beginning". Also in "Finished with computing the " one can drop the "with". Both of these are used a bunch of times.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've changed the messages to have the same structure as the messages of older operations (e.g. OrderBy)

@floriankramer floriankramer merged commit 400e620 into ad-freiburg:master Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants