jayzhan211 commented on PR #26149: URL: https://github.com/apache/datafusion/pull/26149#issuecomment-6245049391
Thanks @txwyy123. The `column_count/64` regression comes from giving every add-only caller (`get_record_batch_memory_size`, hash join build, AsOf, `ExternalSorter`) the refcount map, though none of them release anything. I opened txwyy123/datafusion#1 against your branch with what I'm proposing: 1. `RecordBatchMemoryCounter` stays add-only as on `main`, and release moves to a new `RecordBatchMemoryTracker` built on your `BufferIdMap`, with one shared buffer walk. For primitive columns the counting path compiles to the same instructions as `main`, and timing is within noise once code alignment is controlled. 2. TopK becomes the first caller, which fixes @DamiaoCastro's repro from #26140 (aggregate output → TopK now runs at `-m 275M`, previously needed ~1.5 GB) with neutral end-to-end timing. 3. Tests are trimmed to #26140's acceptance criteria. Could you take a look and merge it into your branch if it makes sense? -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
