westonpace commented on a change in pull request #12702:
URL: https://github.com/apache/arrow/pull/12702#discussion_r834739482
##########
File path: cpp/src/arrow/compute/exec/aggregate_node.cc
##########
@@ -168,17 +168,26 @@ class ScalarAggregateNode : public ExecNode {
Status DoConsume(const ExecBatch& batch, size_t thread_index) {
util::tracing::Span span;
- START_SPAN(span, "Consume",
- {{"aggregate", ToStringExtra()},
- {"node.label", label()},
- {"batch.length", batch.length}});
+ START_SPAN(
+ span, "Consume",
+ {{"aggregate", ToStringExtra()},
+ {"node.label", label()},
+ {"batch.length", batch.length},
+ {"memory_pool_bytes",
+ plan_->exec_context()->memory_pool()->bytes_allocated()},
+ {"memory_used", arrow::internal::tracing::GetMemoryUsed()},
+ {"memory_used_process",
arrow::internal::tracing::GetMemoryUsedByProcess()}});
for (size_t i = 0; i < kernels_.size(); ++i) {
util::tracing::Span span;
- START_SPAN(span, aggs_[i].function,
- {{"function.name", aggs_[i].function},
- {"function.options",
- aggs_[i].options ? aggs_[i].options->ToString() :
"<NULLPTR>"},
- {"function.kind", std::string(kind_name()) + "::Consume"}});
+ START_SPAN(
+ span, aggs_[i].function,
+ {{"function.name", aggs_[i].function},
+ {"function.options",
+ aggs_[i].options ? aggs_[i].options->ToString() : "<NULLPTR>"},
+ {"function.kind", std::string(kind_name()) + "::Consume"},
+ {"memory_pool_bytes",
plan_->exec_context()->memory_pool()->bytes_allocated()},
+ {"memory_used", arrow::internal::tracing::GetMemoryUsed()},
+ {"memory_used_process",
arrow::internal::tracing::GetMemoryUsedByProcess()}});
Review comment:
Given that these memory statistics are process-wide anyways maybe it
makes sense to only enable collection of memory statistics in our own profiling
tools. For example, when running the query tester we can add a span processor
that collects memory statistics.
Then the actual library code would only instrument things specific to the
library.
That only leaves the batching concern but, glancing at the description, and
knowing nothing of the OT spec, I think the span processor itself might batch
its output but its input (the span start/stop calls) won't be batched.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
