wjones127 commented on PR #33731:
URL: https://github.com/apache/arrow/pull/33731#issuecomment-1406775052
A few points I would like feedback on:
* I added `total_bytes_allocated` and `num_allocations`, since they are
metrics available and seem like they would be useful. For example, it might
help remind of us places where we aren't calling Reserve on array builders when
we should be. Does that seem sensible?
* In order to be able to use these metrics for individual benchmarks, we
needed to expose a `ResetStatistics()` method. However, this changes the
semantics of the statistics, since they are no longer guaranteed to represent
the global state since the beginning of the program. That's fine as long as no
one was relying on them for an accurate picture of memory use at the time, but
I do worry it could be a foot-gun.
- An alternative would be to keep a separate optional struct for "local
stats", and allow that to be reset while the global one is left alone. And in
most cases it could be left blank. Does that seem preferable?
* I added memory tracking only on benchmarks that I felt would benefit from
it. For example, I excluded flight because AFAIK many of the allocations happen
in gRPC outside of the memory pool, so this method of measurement wouldn't be
particularly useful there.
--
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]
