andygrove commented on PR #5991: URL: https://github.com/apache/datafusion-comet/pull/5991#issuecomment-5718231377
Fixed in 7ca27dc4d. You're right that gating on a positive value throws away the samples that matter most — a reservation observed at zero is a real value, and allocation sitting above it is exactly the retained-after-release case. The legacy path now gates on whether an allocation sample and a reservation sample have been seen rather than on their being positive, and both of your cases report the 100 MiB. The trace whose only reservation is zero no longer claims there were no pool samples. One correction on the base, though: it doesn't report 100 MiB on that sequence either. Its guard is `latest_jemalloc > 0 && pool_total > 0 && latest_jemalloc > pool_total`, so a zero sum suppresses the comparison there too — I ran both of your sequences against main and it prints 0.0 MB and OK for each. So this is restoring what 83cd46f8 did rather than repairing something I broke relative to main. Doesn't change the fix, but worth pinning down since "preserve" implied the behaviour was there before. I also reworded the legacy warning. It said the excess was understated, which is now only half true: summing inflates the total when a shared pool is counted on every referencing thread and deflates it when a thread hasn't reported yet, so it can err either way. On a 4k-event legacy trace the numbers are still identical to main's, so the divergence is confined to samples where the sum reads exactly zero. And I regenerated the sample output in the tracing doc, since the old block predated the counter and its numbers no longer match what the tool prints. -- 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]
