On Tue, Feb 18, 2020 at 12:58:10PM -0500, Derrick Stolee wrote: > On 2/14/2020 1:22 PM, Jeff King wrote: > > When we do a bitmap-aware revision traversal, we create an object_list > > for each of the "haves" and "wants" tips. After creating the result > > bitmaps these are no longer needed or used, but we never free the list > > memory. > > It's surprising that we got away with this for so long. Is it > possible that this loop of freeing memory could provide significant > performance drawbacks? I'm assuming that the free() loop is much > faster than the malloc() loop, so the impact is negligible. I don't think it's surprising. We're only talking about leaking hundreds or perhaps thousands of structs. So maybe a few kilobytes at most. And we'd typically only run one such bitmap traversal per program. Meanwhile rev-list is generally storing that same set of objects in a pending array, and I think we don't generally clean up after it (nor is there even a convenient function to do so). So I think it's sort of a drop in the bucket of Git's "eh, this pretty much lasts about the whole program anyway" leaks. I don't think there's much performance difference either way in freeing or not. It's mostly just a cleanliness thing. (I do one day dream of being able to run the test suite through a leakchecker, but we're still a ways off, I think). -Peff
