Describe the bug
While profiling with this command (as part of testing out #24035)
I noticed a non trivial amount of time (1% of query) is now spent measuring the memory used in get_record_batch_memory_size
There are at least two memory allocations (for each record batch for each poll for each operator) that add up:
- calling Array::to_data() allocates a vec (for buffers): https://github.com/apache/datafusion/blob/d428760d709a375f3d997c84e9c4748a22584149/datafusion/common/src/utils/memory.rs#L170-L169
- Creating a counted buffer hash table: https://github.com/apache/datafusion/blob/d428760d709a375f3d997c84e9c4748a22584149/datafusion/common/src/utils/memory.rs#L154-L153
To Reproduce
cargo bench --profile=profiling --bench window_query_sql -- "window empty over, aggregate functions"
And then run samply something like
samply record target/profiling/deps/window_query_sql-950028bf4c5b086c --bench "window empty over, aggregate functions"
Expected behavior
I expect memory accounting to be faster:
- Not allocate vec's
- Likely not hash either
I think the overhead is relatively small but since this is called on basically every operator work we could o
Additional context
It looks like it was most recently modified as part of this PR from @jordepic and @2010YOUY01 :
But the problematic allocations existed before that
Describe the bug
While profiling with this command (as part of testing out #24035)
I noticed a non trivial amount of time (1% of query) is now spent measuring the memory used in
get_record_batch_memory_sizeThere are at least two memory allocations (for each record batch for each poll for each operator) that add up:
To Reproduce
cargo bench --profile=profiling --bench window_query_sql -- "window empty over, aggregate functions"And then run samply something like
samply record target/profiling/deps/window_query_sql-950028bf4c5b086c --bench "window empty over, aggregate functions"Expected behavior
I expect memory accounting to be faster:
I think the overhead is relatively small but since this is called on basically every operator work we could o
Additional context
It looks like it was most recently modified as part of this PR from @jordepic and @2010YOUY01 :
But the problematic allocations existed before that